fix(VListItem): allow default color to change when inactive

[Tofandel]

Description

Fixes #9285 by removing the !important keyword in the vuetify style sheet and adding a base-color prop

Motivation and Context

It is currently not possible to change the color of a v-list-item via an hex color="#CCC" prop, it is only possible to do it via a tricky css rule (needs to be very specific, and the selector with this rule is already too specific with an !important keyword,

How Has This Been Tested?

Visually

Before

After

Markup:

<template>
      <v-list dense>
        <v-list-item-group v-model="item">
          <template v-for="(item, i) in items">
          <v-list-item
            :key="i+'0'"
            :value="0"
            v-bind="item"
          >
            <v-list-item-icon>
              <v-icon>settings</v-icon>
            </v-list-item-icon>
            <v-list-item-content>
              <v-list-item-title>normal {{ item }}</v-list-item-title>
            </v-list-item-content>
          </v-list-item>
          <v-list-item
            :key="i+'1'"
            :value="1"
            v-bind="item"
          >
            <v-list-item-icon>
              <v-icon>settings</v-icon>
            </v-list-item-icon>
            <v-list-item-content>
              <v-list-item-title>active {{ item }}</v-list-item-title>
            </v-list-item-content>
          </v-list-item>
          <v-list-item
            dark
            :key="i+'0d'"
            :value="0"
            v-bind="item"
          >
            <v-list-item-icon>
              <v-icon>settings</v-icon>
            </v-list-item-icon>
            <v-list-item-content>
              <v-list-item-title>normal dark {{ item }}</v-list-item-title>
            </v-list-item-content>
          </v-list-item>
          <v-list-item
            dark
            :key="i+'1d'"
            :value="1"
            v-bind="item"
          >
            <v-list-item-icon>
              <v-icon>settings</v-icon>
            </v-list-item-icon>
            <v-list-item-content>
              <v-list-item-title>active dark {{ item }}</v-list-item-title>
            </v-list-item-content>
          </v-list-item>
          </template>
        </v-list-item-group>
      </v-list>
</template>

<script>
export default {
data: () => ({
    item: 1,
    items: [
      { color: "#228822" },
      { color: 'amber' },
      { baseColor: "#228822" },
      { baseColor: 'amber' },
      {}
    ],
  }),
}
</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 added relevant changes to the documentation

[Tofandel]

Should the icon's color change as well maybe? I think it makes more sense

[KaelWD]

Yes, but this was intentional so inactive items aren't highlighted (see https://vuetify-lzjyu0p1p-vuetifyjs1.vercel.app/en/getting-started/installation/)

The text here is supposed to be black unless it's an active route.

[Tofandel]

Okay let me see if I can fix that without an important keyword

[Tofandel]

Hmm, doesn't seem to be possible with css only. Basically they should only get the primary-text class if they are active because that's what's causing this, I'll come back later to this, seems to be a more complex issue

Once the class is removed then this would be enough to avoid color override

[Tofandel]

I think to solve this it would make sense to have a color prop and an active-color prop or, to avoid a breaking change, a base-color prop and color prop (which only applies when the list item is active)

[Tofandel]

I removed the color class when the item is not active, which fixes the demonstrated issue

Also took the liberty of adding the base-color prop which is the prop that controls the color in any state (with color still overriding this if active)

[KaelWD]

This also changes icons to black when inactive:

[Tofandel]

I changed it to only inherit icon color if the base color is set

I also added a colorful example in the doc

The base branch was changed.

[johnleider]

I think the naming is confusing. If we have a visual change based upon the state of an item, it should have a name that reflects that.

Personally, I'd rather have the parent dictate a color and active-color and leave v-list-item alone.

I'm actively working on v3 lists and I'll chime in more as I experiment more.

[KaelWD]

active-color can't be done in v2 without being a breaking change, I'm ok with this as a stopgap then change it again in v3.

I'd rather have the parent dictate a color and active-color and leave v-list-item alone.

There does need to be some way to change the colour of an individual item imo

[KaelWD]

Hey @Tofandel, we discussed this in today's meeting and decided not to implement the new prop in v2 as it would be immediately deprecated. To fix #9285 in v2 we can accept removing the active class when not active though.

[Tofandel]

So just the fix without the base-color prop?

[johnleider]

Can you take a look at v-list as of v3-alpha.5 and compare the styling. We went with the prop name active-color.

[Tofandel]

@johnleider The doc site doesn't seem to include it there https://next.vuetifyjs.com/en/components/application/ any other way to check except building it locally? Did you also add a similar snippet in the doc like: 54f8d4f ? (You can copy it if not, just replace color with activeColor and baseColor with color)

[KaelWD]

@Tofandel I'm going to have to revert this, color no longer affects active route links: #14186

Feel free to open a new PR if you have a solution that works for this case as well. I did try using routable::onRouteChange to set isActive, but that doesn't apply on the initial render and seems to require a parent component to set it back to false (like VTabs does). Another option would be to add the color classes to activeClass, but that only supports named colors.

[Tofandel]

Indeed made a new PR, the onRouteChange should be called on mounted, as for the activeClass to set it back to false no need, you actually just need to call toggle, only if the class boolean is different than the isActive value, effectively trully reflecting isActive on every route change
The toggle also needs to be redefined in the VListItem because it's overriden by the groupable mixin

Probably not the best explanation but you'll understand the code

I also made sure it doesn't change the behavior of tabs and buttons