[go: up one dir, main page]

Skip to content

Improve implementation in Author dropdown on commits page

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

The following discussion from !31686 (merged) should be addressed:

  • @dmishunov started a discussion: (+1 comment)

    Another thing that I've noticed here, @sming-gitlab that was, actually, introduced in another MR of yours. This button has an odd mess with the :disabled props. Technically, it's always set to ON on either the parent DIV or the dropdown itself:

      <div ref="dropdownContainer" v-gl-tooltip :title="tooltipTitle" :disabled="!hasSearchParam">
        <gl-new-dropdown
          :text="dropdownText"
          :disabled="hasSearchParam"
    ...

    Why so? Do I understand right that :disabled on the parent DIV is just to not show the tooltip when hasSearchParam is false? If it' not the case, please, do explain me since I don't understand this.

    If this is the case, however, that's not how we should disable tooltips, I'm afraid: you simply have to return some falsey value as the tooltip text (undefined, null, ''). But the irony is that false itself (as it turns out in your check when this.hasSearchParam is false) doesn't do what you want: it simply does not set the title attribute to the parent DIV (in HTML: <div>...</div>) that, having the directive on this element, still generates the tooltip even though an empty one. What you need is to still keep the title attribute but make sure it doesn't have any value (in HTML: <div title>...</div>): this is the scenario to prevent the tooltip from being shown.

    Here is the patch to better explain the correct handling of "not showing" the tooltip:

    Index: app/assets/javascripts/projects/commits/components/author_select.vue
    IDEA additional info:
    Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
    <+>UTF-8
    ===================================================================
    --- app/assets/javascripts/projects/commits/components/author_select.vue	(revision 697d0734f1ae469a9a3522838e36b435d7cdf0be)
    +++ app/assets/javascripts/projects/commits/components/author_select.vue	(date 1589358185344)
    @@ -47,7 +47,7 @@
           return this.currentAuthor || __('Author');
         },
         tooltipTitle() {
    -      return this.hasSearchParam && tooltipMessage;
    +      return this.hasSearchParam? tooltipMessage: '';
         },
       },
       mounted() {
    @@ -106,7 +106,7 @@
     </script>
     
     <template>
    -  <div ref="dropdownContainer" v-gl-tooltip :title="tooltipTitle" :disabled="!hasSearchParam">
    +  <div ref="dropdownContainer" v-gl-tooltip :title="tooltipTitle">
         <gl-new-dropdown
           :text="dropdownText"
           :disabled="hasSearchParam"
    

    Please, feel free to resolve this issue either in this MR, or create a follow-up to handle this issue. Thanks 😄

Edited by 🤖 GitLab Bot 🤖