Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix(VMenu): wrong item highlighted #14972

Open
wants to merge 6 commits into
base: v2-stable
Choose a base branch
from

Conversation

kglazier
Copy link

@kglazier kglazier commented Apr 20, 2022

Description

Fix Wrong item highlighted when using prepend-item slot on v-select. Exclude prepend items from the list of options by returning only valid selection options for highlight. Don't highlight option when deselecting.

Motivation and Context

resolves #14489

How Has This Been Tested?

Manually tested using the playground.vue code below. This code is taken from the vuetify select component page
https://vuetifyjs.com/en/components/selects/#append-and-prepend-item

Markup:

<template>
  <div>
    <v-row justify="space-around">
      <v-switch
        v-model="dense"
        label="Dense"
      ></v-switch>
      <v-switch
        v-model="selectable"
        label="Selectable"
      ></v-switch>
      <v-switch
        v-model="activatable"
        label="Activatable"
      ></v-switch>
      <v-switch
        v-model="hoverable"
        label="Hoverable"
      ></v-switch>
      <v-switch
        v-model="shaped"
        label="Shaped"
      ></v-switch>
      <v-switch
        v-model="rounded"
        label="Rounded"
      ></v-switch>
      <v-switch
        v-model="openOnClick"
        label="Open on any item click"
      ></v-switch>
      <v-col cols="12">
        <v-select
          v-model="selectedColor"
          :items="selectedColors"
          :disabled="!selectable"
          label="Selected checkbox color"
        ></v-select>
      </v-col>
      <v-col cols="12">
        <v-select
          v-model="color"
          :items="selectedColors"
          :disabled="!activatable"
          label="Active node color"
        ></v-select>
      </v-col>
    </v-row>

    <v-treeview
      :items="items"
      :dense="dense"
      :selectable="selectable"
      :activatable="activatable"
      :hoverable="hoverable"
      :open-on-click="openOnClick"
      :selected-color="selectedColor"
      :color="color"
      :shaped="shaped"
      :rounded="rounded"
    ></v-treeview>
  </div>
</template>

<script>
  export default {
    data: () => ({
      items: [
        {
          id: 1,
          name: 'Applications :',
          children: [
            { id: 2, name: 'Calendar : app' },
            { id: 3, name: 'Chrome : app' },
            { id: 4, name: 'Webstorm : app' },
          ],
        },
        {
          id: 5,
          name: 'Documents :',
          children: [
            {
              id: 6,
              name: 'vuetify :',
              children: [
                {
                  id: 7,
                  name: 'src :',
                  children: [
                    { id: 8, name: 'index : ts' },
                    { id: 9, name: 'bootstrap : ts' },
                  ],
                },
              ],
            },
            {
              id: 10,
              name: 'material2 :',
              children: [
                {
                  id: 11,
                  name: 'src :',
                  children: [
                    { id: 12, name: 'v-btn : ts' },
                    { id: 13, name: 'v-card : ts' },
                    { id: 14, name: 'v-window : ts' },
                  ],
                },
              ],
            },
          ],
        },
        {
          id: 15,
          name: 'Downloads :',
          children: [
            { id: 16, name: 'October : pdf' },
            { id: 17, name: 'November : pdf' },
            { id: 18, name: 'Tutorial : html' },
          ],
        },
        {
          id: 19,
          name: 'Videos :',
          children: [
            {
              id: 20,
              name: 'Tutorials :',
              children: [
                { id: 21, name: 'Basic layouts : mp4' },
                { id: 22, name: 'Advanced techniques : mp4' },
                { id: 23, name: 'All about app : dir' },
              ],
            },
            { id: 24, name: 'Intro : mov' },
            { id: 25, name: 'Conference introduction : avi' },
          ],
        },
      ],
      dense: false,
      selectable: false,
      activatable: false,
      hoverable: false,
      openOnClick: false,
      shaped: false,
      rounded: false,
      color: 'primary',
      selectedColor: 'accent',
      selectedColors: [
        'accent',
        'teal',
        'red',
        'success',
        'warning lighten-2',
      ],
    }),
  }
</script>

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Improvement/refactoring (non-breaking change that doesn't add any features but makes things better)

Checklist:

  • The PR title is no longer than 64 characters.
  • The PR is submitted to the correct branch (master for bug fixes and documentation updates, dev for new features and backwards compatible changes and next for non-backwards compatible changes).
  • My code follows the code style of this project.
  • I've added relevant changes to the documentation (applies to new features and breaking changes in core library)

@KaelWD KaelWD changed the title Fix(VMenu): wrong item highlighted #14957 Fix(VMenu): wrong item highlighted Apr 21, 2022
Copy link
Member

@KaelWD KaelWD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change removes focus highlighting for non-selected items (try using a menu with just the keyboard). It also doesn't fully fix the problem, some items are still skipped because tiles includes the divider but filteredTiles doesn't (select the first three items then use arrow keys).

Posted playground is for treeview, here's one from the linked issue with a regular menu added too:

<template>
  <v-app>
    <v-container>
      <v-select
        v-model="selectedFruits"
        :items="fruits"
        label="Favorite Fruits"
        multiple
      >
        <template #prepend-item>
          <v-list-item
            ripple
            @click="toggle"
          >
            <v-list-item-action>
              <v-icon :color="selectedFruits.length > 0 ? 'indigo darken-4' : ''">
                {{ icon }}
              </v-icon>
            </v-list-item-action>
            <v-list-item-content>
              <v-list-item-title>
                Select All
              </v-list-item-title>
            </v-list-item-content>
          </v-list-item>
          <v-divider class="mt-2"></v-divider>
        </template>
        <template #append-item>
          <v-divider class="mb-2"></v-divider>
          <v-list-item disabled>
            <v-list-item-avatar color="grey lighten-3">
              <v-icon>
                mdi-food-apple
              </v-icon>
            </v-list-item-avatar>

            <v-list-item-content v-if="likesAllFruit">
              <v-list-item-title>
                Holy smokes, someone call the fruit police!
              </v-list-item-title>
            </v-list-item-content>

            <v-list-item-content v-else-if="likesSomeFruit">
              <v-list-item-title>
                Fruit Count
              </v-list-item-title>
              <v-list-item-subtitle>
                {{ selectedFruits.length }}
              </v-list-item-subtitle>
            </v-list-item-content>

            <v-list-item-content v-else>
              <v-list-item-title>
                How could you not like fruit?
              </v-list-item-title>
              <v-list-item-subtitle>
                Go ahead, make a selection above!
              </v-list-item-subtitle>
            </v-list-item-content>
          </v-list-item>
        </template>
      </v-select>

      <v-menu>
        <template #activator="{ props, on }">
          <v-btn
            v-bind="props"
            v-on="on"
          >Plain menu</v-btn>
        </template>
        <v-list>
          <v-list-item
            v-for="i in fruits"
            :key="i"
            :value="i"
            @click="clicked(i)"
          >
            <v-list-item-title>{{ i }}</v-list-item-title>
          </v-list-item>
        </v-list>
      </v-menu>
    </v-container>
  </v-app>
</template>

<script>
  export default {
    data: () => ({
      fruits: [
        'Apples',
        'Apricots',
        'Avocado',
        'Bananas',
        'Blueberries',
        'Blackberries',
        'Boysenberries',
        'Bread fruit',
        'Cantaloupes (cantalope)',
        'Cherries',
        'Cranberries',
        'Cucumbers',
        'Currants',
        'Dates',
        'Eggplant',
        'Figs',
        'Grapes',
        'Grapefruit',
        'Guava',
        'Honeydew melons',
        'Huckleberries',
        'Kiwis',
        'Kumquat',
        'Lemons',
        'Limes',
        'Mangos',
        'Mulberries',
        'Muskmelon',
        'Nectarines',
        'Olives',
        'Oranges',
        'Papaya',
        'Peaches',
        'Pears',
        'Persimmon',
        'Pineapple',
        'Plums',
        'Pomegranate',
        'Raspberries',
        'Rose Apple',
        'Starfruit',
        'Strawberries',
        'Tangerines',
        'Tomatoes',
        'Watermelons',
        'Zucchini',
      ],
      selectedFruits: [],
    }),

    computed: {
      likesAllFruit () {
        return this.selectedFruits.length === this.fruits.length
      },
      likesSomeFruit () {
        return this.selectedFruits.length > 0 && !this.likesAllFruit
      },
      icon () {
        if (this.likesAllFruit) return 'mdi-close-box'
        if (this.likesSomeFruit) return 'mdi-minus-box'
        return 'mdi-checkbox-blank-outline'
      },
    },

    methods: {
      toggle () {
        this.$nextTick(() => {
          if (this.likesAllFruit) {
            this.selectedFruits = []
          } else {
            this.selectedFruits = this.fruits.slice()
          }
        })
      },
      clicked (val) {
        console.log(val)
      },
    },
  }
</script>

Fix wrong item higlighted when using prepend-item slot on v-select.  Include prepend and append
items when determining the index.

vuetifyjs#14972
Fix wrong item highlight when using prepend-item slot on v-select. Include prepnd and append items
when determining the index.

vuetifyjs#14972
Fix wrong item highlight when using prepend-item slot on v-select. Include prepnd and append items
when determining the index.

vuetifyjs#14972
@kglazier
Copy link
Author

Retested with keyboard and with mouse on both treeview and regular menu.

@joel-wenzel
Copy link
Contributor

Was this PR still waiting on changes?

@kglazier
Copy link
Author

kglazier commented Sep 6, 2022

No more changes have been requested. I requested another review.

@joel-wenzel
Copy link
Contributor

@KaelWD Do you have time to take another look at the changes?

@KaelWD KaelWD changed the base branch from master to v2-stable February 5, 2023 09:54
@MajesticPotatoe MajesticPotatoe added T: bug Functionality that does not work as intended/expected C: VMenu VMenu labels Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: VMenu VMenu T: bug Functionality that does not work as intended/expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug Report][2.6.1] v-select with prepended/appended items highlights incorrect item
5 participants