Thu, Jun 17
I wrote a sketch of a dialog component, before abandoning it and working on other things: https://gerrit.wikimedia.org/r/c/wvui/+/699788 . It's very incomplete, but here are some of the things I had in mind while writing this.
Tue, Jun 15
Mon, Jun 14
I like the idea of reducing compatibility problems by only exposing exports to package modules. That should be pretty safe, because AFAICT the scenarios are:
- Code uses exports as an implicit global: this shouldn't be happening, eslint would catch it
- Code uses exports as a declared variable or as a function parameter: this is fine, it'd just shadow the exports parameter from the wrapper function that we'd provide
- Code uses typeof exports or similar to detect its presence: I think it's safe to assume that only UMD code would do this, and the wrapper file would need to be changed in the way you describe
For performance reasons, we'd like to avoid exposing the entire set of icons, because it's so large. Instead, I think we'll want to offer two options:
- Using a build step, tree-shake just the icons that are needed, and include those in the built bundle
- Allow RL modules to list the icons they need in their module definition, and build some ResourceLoaderImageModule-style magic to automatically add wvuiIcons.iconName = '...svg string...'; to those modules
Fri, Jun 11
Point 1: agreed.
Thu, Jun 10
Hello security people (@sbassett @Mstyles), just a heads up that I'll be on vacation from right after all-hands until pretty much the end of the quarter (June 18 - 29, returning on June 30). If you have questions about this task during that time, please ping @egardner.
Wed, Jun 9
T280828: Create a temporary entry for all WVUI components is also relevant for how these two bundles came to be
Tue, Jun 8
I found this example in the ARIA docs that seemed to more closely track how our dropdown works, so I implemented its behavior instead. The significant difference is that I'm not setting role="combobox" (because that's only supposed to be set on input[type="text"] elements) and that I'm setting aria-activedescendant on the listbox instead of on (what used to be) the combobox. I found it strange that this example doesn't link the head (button element in their case) of the dropdown to the list, so I used aria-owns the way it's used in the combobox example.
Mon, Jun 7
If we did this, we would also be able to get rid of wrapper files for UMD distributions of external libraries, like moment-module.js. UMD does assign to module.exports rather than to exports, but it uses typeof exports !== 'object' && typeof module !== 'undefined' to detect whether it's going to assign to module.exports or try something else. In our environment, UMD ends up assigning to a global variable.
Re Vue versions, with our current plans as I understand them, everything's going to have to be Vue 2 for the time being. But we should definitely develop guidelines for things to do and things to avoid that will make a future Vue 3 migration easier.
Fri, Jun 4
Yes, I definitely use Storybook a lot, both during initial development and when making changes to components.
It's not just you, this is legitimately confusing, and I don't know what all of these widgets are either. I think we'll suffer from this problem much less in WVUI because 1) we'll be more careful to choose non-confusing names now that we've been burned by this issue, and 2) we'll need fewer components because a) we're not using inheritance and b) we're using data rather than individual components for options.
@Jdforrester-WMF tells me this is blocked on upgrading the version of Node used in our CI images, which is in turn blocked on SRE approving a newer version of Node (at least for pipeline jobs like the one used for WVUI, which uses a production image)
Thu, Jun 3
Here are some other things I thought about and/or would like reviewers of my patch to provide input on:
- What do you think of my use of slots? Should OptionsMenu have a default implementation for its slot, or can it afford not to have one, since it's an internal component? What do you think of the use of named slots with default behavior in Dropdown?
- Right now I have only one type for options (OptionMenuItem). Should there instead be a base type (OptionMenuItem) that only has what it needs (ID and disabled; label is not needed if there is no default rendering, or if the default rendering is the ID), and a specific type (DropdownItem) that extends that and adds dropdown-specific things (e.g. group header, icon)? Or are those things not really dropdown-specific and should they be supported in all components that use OptionMenu?
- The MediaSearch implementation accepts various shorthand formats for the items prop. Should we do that / add that here too, and normalize to the longhand format internally?
- What do you think of my use of v-model with custom prop and event names? Should I rename to modelValue/input so that I can use the modelWrapper composable? Or should the composable support alternative prop/event names? Should the composable use generics so that it can be used with typed props?
- Should there be styling for the default label that is shown when nothing is selected (e.g. italics)? (There's already a --value-selected CSS class that could be used for this, following MediaSearch's example.)
- How should focus be handled? The Wikit and WVUI implementations allow the dropdown gain focus when clicked, which makes it easy to hide the menu on blur and to capture key events, but because they don't prevent mousedown the text in the dropdown can be selected. The OOUI implementation prevents mousedown, so there are no selection issues, but that prevents the dropdown from gaining focus (except through the keyboard), so key events and click-away events have to be captured on the document. I chose to go the former route, but maybe we should prevent the text from being selected and prevent the dropdown from visually appearing as if it's focused unless it was focused via the keyboard?
- Which ARIA attributes are needed that I missed? I haven't added the ones from MediaSearch that rely on IDs. MediaSearch uses the name prop to build IDs, but I don't like the idea of making name a required prop (form field names also don't have to be document-unique, only unique within a form). But we could also generate random IDs at mount time and use those, if the ID-based ARIA attributes are useful.
I agree, and my proposed patch calls the specific UI component a "Dropdown", while calling the more generic menu an "OptionsMenu".
Wed, Jun 2
So sorry for the late response, @Mstyles! I missed your comment and only saw it today because I was working on this task for other reasons. I've added it to our team's workboard so that we can keep a closer eye on it in the future.
Tue, Jun 1
Wed, May 26
Another trick I learned yesterday: if you access a ref, like this.$refs.foo, TypeScript doesn't know exactly what type it is, and infers its type as Vue | Element | Vue | Element. That's not a very useful type if you want to call methods or access properties, but you can tell TypeScript what the right type is.
May 25 2021
One thing I've noticed while doing development in WVUI is that the TypeScript integration with Vue 2 behaves strangely sometimes. For example, it only works if you explicitly specify the return type for every computed property function. If you don't specify a return type on any one computed property function that accesses this.something, then type inference breaks for all props and computed properties in all computed property functions.
May 15 2021
With this patch:
May 14 2021
May 13 2021
@egardner and I made a proof of concept that integrates Vite with ResourceLoader to provide hot module reloading for Vue code. It turned out to be simpler to achieve than I expected. The extension implementation is here, and the associated core patch is here.
May 12 2021
This is a good point, and I had forgotten to say: the little extra annoyance is probably worth it for performance reasons (so that tree shaking works), and I also like your framing here as "strong typing for icons", that's exactly what it is. It's also worth noting that, in cases where logic is needed to choose which icon is displayed, you need a computed function anyway, and at that point returning an Icon object instead of a string isn't any extra work.
I'd be curious to hear what other people's experiences are with these icon systems or others, what suggestions they have for what our icon system should look like, and what other factors they think we should consider in the architecture of our icon system.
Here are some pros/cons and trade-offs with WVUI's current icon system as I see them:
May 11 2021
FYI you can already do this without a startIcon prop, because a button (unlike an input) can contain arbitrary content:
May 8 2021
May 5 2021
May 3 2021
developers will benefit from better IDE tooling and will be able to write code in a more standardized way
What do you mean by this exactly? If you mean import and export, then I think there would be additional challenges in getting those to work/coexist with ResourceLoader. Are there other language features that are only available when type="module" is used?
Apr 30 2021
Apr 29 2021
The deployment target date is nuanced: we plan to introduce the plugin in WVUI in the coming weeks, but that doesn't immediately result in production deployment. That would only occur once we cut a new release of WVUI and put it in MediaWiki core. We may well be ready to do that some time in May or early June, but if the security review isn't complete by then, we could hold off (or put it behind a feature flag) until we have real code in an extension using it that is ready to deploy. And we don't think we would get to _that_ stage until July (or perhaps June if things go well/fast).
See T281527: Security Readiness Review For Vue composition API plugin for more context on how this task came about.
Thanks @sbassett! I asked the other engineers on the Vue team, and it sounds like we do plan on using the composition API plugin soon (which is a plugin for Vue 2.6 that essentially adds a subset of the anticipated 2.7 features). I've opened T281527: Security Readiness Review For Vue composition API plugin to request a review of that plugin. I'll link this task there too for context. We'll plan to reopen this task when Vue 2.7 comes out.
Since it looks like Vue is never going to support IE11 in Vue 3, and instead backport some Vue 3 features to a new Vue 2.7 release (that's what's currently being proposed, and it seems likely to happen), we don't think that any WMF code will likely migrate to Vue 3 for at least a year or more. I do think we'll likely upgrade to 2.7 once it's out, and we may want to use the composition API plugin before then, to get some of those 2.7 features early (the proposal calls for this plugin to be made part of Vue itself in 2.7). So I think a security readiness review of Vue 3 can be postponed for now, and we'll probably ask for a review of Vue 2.7 when it comes out. We may also ask for a review of the composition API plugin for 2.6, I'll ask the other Vue migration team members about that.
Apr 28 2021
Apr 26 2021
Everything you asked about is correct, as far as I know
Apr 23 2021
The attached patch proposes an implementation using approach #2 from the list above. I'm not 100% confident that this is the right approach, but it seemed like the one with the fewest downsides to me. Eager to get feedback on this.
Apr 22 2021
Apr 21 2021
Apr 20 2021
Apr 19 2021
This color is @background-color-quiet--active from wikimedia-ui-base. On a white background, this color looks the same as Base80, which is what the style guide prescribes.
Apr 8 2021
I like the idea of putting the label in the main slot. Speaking of labels, what are your thoughts on how we should render those? Wikit generates a random ID, then uses <input type="checkbox" id="randomValue" /><label for="randomValue">Label text</label>. MediaSearch puts the input tag inside of the label tag (<label><input type="checkbox" />Label text</label>), which avoids the need for an ID. For that reason, I lean towards the latter.
We should also think about how to handle toggle buttons and toggle switches. I don't think we have Vue implementations of these yet, but OOUI has them. Toggle buttons look exactly like normal buttons, except that they stay pressed after you click them (and become unpressed again when you click them again). We could consider adding a prop to the regular button component that enables this mode. Toggle switches are a bit more akin to checkboxes (see T279714), and I think it would make sense for them to be a separate component.
Mar 30 2021
Mar 26 2021
Mar 19 2021
Another issue is that the Vite loader assumes ES6 modules are used. It uses dynamic import (await import( 'url' )) to load new versions of modules, which is an ES2020 feature. The latter is fine, we can assume that developers use recent browser versions. The former is a problem, not necessarily because of browser requirements (basic import/export is in ES6, although we probably wouldn't want to limit HMR to ES6-only modules) but because ResourceLoader doesn't support ES6 modules and would require fundamental changes to support them (for example, import/export can only be used on the top level, so wrapping module contents in functions breaks them).
Some not-fully-organized notes from digging into how HMR works in Vite, and how we might integrate it into RL:
Mar 17 2021
- Risky Patch! 🚂🔥
As other commenters have pointed out, async/await is part of ES2017, whereas the minifier only supports ES6 (ES2015) currently. Adding support for async and other post-ES6 language features to the minifier should be pretty easy. However, making those available for MediaWiki code also involves a trade-off: are the additional features worth excluding more browsers? This depends on how useful the new features are, and how much usage the non-supporting browsers have. I've started a task for exploring this here: T277675: Explore native support for ES2016 (ES7) or higher versions
I haven't yet looked at which browsers support these features and how well-used they are. That information should be added to this task next. I don't have time to do that right now; anyone else should feel free to pick that up.