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(VIcon): allow colons to be used in icon names #14038

Merged
merged 3 commits into from
Aug 24, 2021

Conversation

glen-84
Copy link

@glen-84 glen-84 commented Aug 8, 2021

Description

Implements the suggestion by @nekosaur in #13975 – match the start of an icon string with the registered icon sets, instead of assuming that any string containing a colon includes an icon set name.

Fixes #13975. (hopefully – not yet tested [see below])

Motivation and Context

See #13975. It allows icons to be URLs.

How Has This Been Tested?

There is no test file for the icon composable. I tried to add one, but I got a bit stuck:

// Composables
import { useIcon } from '../icons'

describe('icons.ts', () => {
  it.each([
    // Default icon set.
    ["success", {component: ???, icon: "success"}],
    ["https://example.com/icon.svg", {component: ???, icon: "https://example.com/icon.svg"}],
    // MDI icon set.
    ["mdi:success", {component: ???, icon: "success"}],
    // Custom icon set.
    ["custom:https://example.com/icon.png", {component: ???, icon: "https://example.com/icon.png"}],
  ])('should return the correct icon set and name', (props, expected) => {
    const { iconData } = useIcon(props)

    expect(iconData.value.component).toEqual(expected.component)
    expect(iconData.value.icon).toEqual(expected.icon)
  })
})

I'm not sure how to configure/reference the icon set components.

Markup:

// Paste your FULL Playground.vue here

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)

@glen-84 glen-84 added E: icons Icons composable T: bug Functionality that does not work as intended/expected labels Aug 8, 2021
@glen-84 glen-84 requested a review from nekosaur August 8, 2021 12:36
Copy link
Member

@nekosaur nekosaur left a comment

Choose a reason for hiding this comment

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

Here's how you could test

// Composables
import { defineComponent } from 'vue'
import { mount } from '@vue/test-utils'
import { createVuetify } from '@/framework'
import { useIcon } from '../icons'

declare const expect: jest.Expect
declare const it: jest.It

describe('icons', () => {
  const Component = defineComponent({
    props: {
      icon: String,
    },
    setup (props) {
      const { iconData } = useIcon(props)

      return () => iconData.value.icon
    },
  })

  const vuetify = createVuetify({
    icons: {
      defaultSet: 'mdi',
      sets: {
        custom: {
          component: () => null,
        },
      },
    },
  })

  it.each([
    // Default icon set.
    ['success', 'success'],
    ['https://example.com/icon.svg', 'https://example.com/icon.svg'],
    // MDI icon set.
    ['mdi:success', 'success'],
    // Custom icon set.
    ['custom:https://example.com/icon.png', 'https://example.com/icon.png'],
  ])('should return the correct icon name', (icon, expected) => {
    const wrapper = mount(Component, {
      props: {
        icon,
      },
      global: {
        plugins: [vuetify],
      },
    })

    expect(wrapper.text()).toEqual(expected)
  })
})

packages/vuetify/src/composables/icons.tsx Outdated Show resolved Hide resolved
@glen-84 glen-84 requested a review from nekosaur August 8, 2021 17:55
@KaelWD KaelWD changed the title fix(icons.tsx): allow colons to be used in icon names fix(icons): allow colons to be used in icon names Aug 8, 2021
@glen-84 glen-84 added this to the v3.0.0 milestone Aug 11, 2021
@KaelWD KaelWD changed the title fix(icons): allow colons to be used in icon names fix(VIcon): allow colons to be used in icon names Aug 24, 2021
@KaelWD KaelWD merged commit cb956f2 into vuetifyjs:next Aug 24, 2021
@glen-84 glen-84 deleted the icon-set-matching branch September 15, 2021 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E: icons Icons composable T: bug Functionality that does not work as intended/expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants