Sharing notes:

To add the new check icon:

• Add a new prop called checkIcon (or something else).
  • Refer to the icon prop [see code snippet]. The icon can be any icon based on what the user sets the icon's value to be. The checkIcon will be implemented differently because we know the checkIcon value never changes (meaning the checkIcon's icon remains the same).

icon: {
	type: [ String, Object ] as PropType<Icon>,
	default: ''
},

• • The checkIcon will have a different type and default value from icon.
  • The checkIcon prop determines whether to display a check icon for the multi-select menu item. The checkIcon prop's type is Boolean and default value is false.
• Add a cdx-icon to the template(where the HTML is). The icon will be conditionally rendered using v-if. We'll set icon's value in the template.
• Style and align the icon to match the Figma spec.
  • To align the icon, we can do approach A or B:
    • (A) Use the Less mixin (icon-alignment.less). This is what's done in TextInput and TextArea.
    • (B) Use flexbox to wrap the existing contents (everything but the new check icon) in a <span>. Then inside content styles, add justify-content: space-between; to create space between content and the check icon.

Please feel free to correct me and make other suggestions.

Hey @lwatson, let's chat before implementation begins on this task. I don't think we should use a prop here, and I don't think anything should be customizable by the user. Let's discuss when you are back.

Hi @egardner, I would like to give this a shot. Can I go ahead and assign it to myself?

In T371998#10114762, @Nunya wrote:

Hi @egardner, I would like to give this a shot. Can I go ahead and assign it to myself?

Hi @Nunya, sure go for it! There is a linked Figma file in the task description which contains design mock-ups – let me know if you have trouble accessing it.

Per my earlier comment, this icon should not require the addition of new props – it should just automatically appear in selected MenuItems when a multi-select menu is being used. You might find it helpful to review T368022 (which has been merged and will go out in tomorrow's release) – this is the foundation that this task will build on. The Menu component now includes an isMultiSelect computed property which you can consult to determine whether or not a given menu supports multiple selections. This information is passed down as a prop to the MenuItem components inside a menu, so it should already be available where you need it.

If you haven't done so already, I would recommend that you take a look at the MW Gerrit tutorial as well (this is a linked from the "How to Become a MediaWiki Hacker" page). This information will be useful when submitting patches to Codex. The general workflow should work like this:

1. Ensure that Gerrit is set up in your local copy of Codex (using git review -s; see tutorial for more info). You need to install git review if you don't have it already.
2. Check out a new branch when working locally (I often use the task number for this) – git checkout -b T371998, etc
3. Submit the patch when you are ready, with a commit message that includes a line at the end that looks like: Bug: T371998 – this will allow phabricator to connect the task here with the patch in gerrit. Use the git review command for this
4. If you need to make updates, you can first download the remote version (to ensure you have any other changes that have been made): git review -d your_patch_number
5. When updating an existing patch, use git commit --amend to update the existing commit instead of making a new one. Then re-submit it using git review -R.

Feel free to comment here if you have questions and I can help troubleshoot. Thanks for volunteering!

Change #1070581 had a related patch set uploaded (by Bmartinezcalvo; author: Bmartinezcalvo):

[design/codex@main] docs: update menu item guidelines

https://gerrit.wikimedia.org/r/1070581

Hey @Nunya have you had a chance to work on this? If not I'll pick this up tomorrow

Hey @lwatson just at a glance I saw a Pacth-in-review badge on the ticket so I thought someone already submitted a patch for it. Sorry about that, you can pick it up since you would be more familiar with the codebase and can put in the change fast enough. I will look for something else I can contribute to, sorry for the delay.

Hey @Nunya, thanks for flagging that! The patch in review includes updates to the Codex docs based on the changes we need to make to MenuItem, so I can see how that could have caused some confusion.

I’ll go ahead and work on the task since it’s part of our current sprint (DST-Sprint-31 (2024-09-03 to 2024-09-13)). I appreciate your willingness to contribute, and encourage you to pick up another task! If you have any questions or if there's a specific area of interest you'd like to focus on, feel free to reach out to me or the team. We’re happy to support you!

Change #1071721 had a related patch set uploaded (by LWatson; author: LWatson):

[design/codex@main] MenuItem: multiselect check icon

https://gerrit.wikimedia.org/r/1071721

Sure thanks @lwatson @egardner for the support!

@lwatson I've reviewed the new multi-selection prop included within the MenuItem and it looks god and works fine in the Menu with multi-select. I would just update the MenuItem configurable demo to be more intuitive, following the natural sequence of interacting with the MenuItem. So I suggest this order of the states:

States

highlighted (could this be renamed to "hover" instead?)
active
selected
multi-select (place this below the selected)
disabled

And, if possible, I would also reorganize these other props as following:

icon (since it's visually before the label, let's place it before label in the configurable demo as well)
label
boldLabel (let's place this below the label since both are related)
match
supportingText
description
hideDescriptionOverflow (could this be renamed to "descriptionTruncation"?)
searchQuery (since this is a more complex prop, I would place it last, and I wonder if we actually need to show it in the configurable demo)

Change #1072254 had a related patch set uploaded (by LWatson; author: LWatson):

[design/codex@main] docs: reorder MenuItem's config demo props

https://gerrit.wikimedia.org/r/1072254

In T371998#10138155, @gerritbot wrote:

Change #1072254 had a related patch set uploaded (by LWatson; author: LWatson):

[design/codex@main] docs: reorder MenuItem's config demo props

https://gerrit.wikimedia.org/r/1072254

As requested @bmartinezcalvo, I reordered the props listed in the config demo. I did not rename any props.

Change #1071721 merged by jenkins-bot:

[design/codex@main] MenuItem: multiselect check icon

https://gerrit.wikimedia.org/r/1071721

Change #1072254 merged by jenkins-bot:

[design/codex@main] docs: reorder MenuItem's config demo props

https://gerrit.wikimedia.org/r/1072254

El T371998#10138177, @lwatson escribió:

In T371998#10138155, @gerritbot wrote:

Change #1072254 had a related patch set uploaded (by LWatson; author: LWatson):

[design/codex@main] docs: reorder MenuItem's config demo props

https://gerrit.wikimedia.org/r/1072254

As requested @bmartinezcalvo, I reordered the props listed in the config demo. I did not rename any props.

@lwatson thank you! The configurable demo looks much more intuitive now.

Change #1070581 merged by jenkins-bot:

[design/codex@main] docs: update menu item guidelines

https://gerrit.wikimedia.org/r/1070581

Change #1073562 had a related patch set uploaded (by LWatson; author: LWatson):

[mediawiki/core@master] Update Codex from v1.12.0 to v1.13.0

https://gerrit.wikimedia.org/r/1073562

Test wiki created on Patch demo by LWatson-WMF using patch(es) linked to this task:
http://patchdemo.wmcloud.org/wikis/ceebb67189/wiki/Main Page