Page MenuHomePhabricator

Build the wvui-icon component
Closed, ResolvedPublic

Description

Referencing T255902 for guidance, build the wvui-icon component for Vue.js search.

Acceptance criteria

Component can be used in slot content.
All component acceptance criteria in T255902 are covered.

PR and extended discussion on GitHub

https://github.com/wikimedia/wvui/pull/47

Event Timeline

Niedzielski added subscribers: AnneT, Niedzielski.

@Volker_E, I think this task should be assigned to @AnneT, right?

Sounds right to me. And I hope for @AnneT too. :)

I haven't been following the development of this component super closely, so forgive me for asking a bit of a naive question here.

Right now the Icon component expects a raw SVG path to be provided to it as a property. For convenience a bunch of pre-defined variables have been provided in the themes directory which can be imported at the point of use (and if you are using webpack, you can tree-shake the unused icons out of your compiled code); otherwise you can supply a custom SVG path on the fly if you need something special.

Would it make sense to support another use case where the user just passes in a string key name instead? <icon :name="'search'"></icon> or something similar? In my mind this would be simpler for the end user and would provide better encapsulation (the parent component doesn't need to know or care about SVG paths in this case; only the Icon itself does). The ability to pass in a raw SVG string could then become more of an escape hatch for edge cases.

In this case you'd still need a pre-defined set of icons to reference (SVG paths as well as language-specific exceptions, etc). This might be a good use-case for a Vue plugin. Any given Vue instance could be initialized with an icons plugin that is given a list of all the icons to include. Then the Icon component could check for the provided key in this.$icons and extract the appropriate SVG path, language rules, etc.

(I should add I'm thinking particularly about the usage in non-Webpack extensions here, which I know is outside the immediate scope of the search project but probably still worth thinking about)

@Volker_E I used element-to-path for most of them. There were a handful of icons that weren't exactly right after the conversion and @mwilliams kindly helped me by using Illustrator to export SVG files with a single path.

I'd like to resolve this task.

Going through the pull request again, I think we need to follow-up with

  • documentation on how to add/update icons
  • evaluate if having the icons as explicit SVGs (which would make quick lookup and transfer simpler IMO) is useful and rightfully done so in WVUI repo
  • if any of @santhosh's customizations via controls are wishful for the library or a theme currently?

Anything that you'd want to see here as well, @AnneT, @egardner or @santhosh?

I'd like to resolve this task.

Going through the pull request again, I think we need to follow-up with

  • documentation on how to add/update icons
  • evaluate if having the icons as explicit SVGs (which would make quick lookup and transfer simpler IMO) is useful and rightfully done so in WVUI repo
  • if any of @santhosh's customizations via controls are wishful for the library or a theme currently?

Anything that you'd want to see here as well, @AnneT, @egardner or @santhosh?

{{ping}} can this be resolved? The patch on github was merged

In my opinion, we should close this task in favor of T260815, and either expand upon that task or open a new task if/when we decide to overhaul the existing Icon component. @Volker_E, what do you think?