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

Update DropdownMenu readme #13410

Merged
merged 5 commits into from
Jan 22, 2019
Merged

Update DropdownMenu readme #13410

merged 5 commits into from
Jan 22, 2019

Conversation

melchoyce
Copy link
Contributor

Updating documentation to describe the use and functionality of the DropdownMenu component.

Thanks to @sarahmonster, @jasmussen, and @kjellr for helping draft this.

Adding documentation to describe the use and functionality of the DropdownMenu component.

Thanks @sarahmonster and @jasmussen for drafting this.
@melchoyce melchoyce added the [Type] Developer Documentation Documentation for developers label Jan 21, 2019
@melchoyce melchoyce requested a review from kjellr January 21, 2019 21:00

## Usage
![An expanded DropdownMenu, containing a list of MenuItems.](https://wordpress.org/gutenberg/files/2019/01/DropdownMenuExample.png)
Copy link
Member

Choose a reason for hiding this comment

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

I see that is a common way of handling images for docs. I'm wondering if it wouldn't be better to keep them on GitHub instead. This would make it open for everyone. At the moment there is a very small group of maintainer who can override the existing files or add new ones.

Copy link
Contributor

Choose a reason for hiding this comment

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

That would definitely be easier to update — the reason we've been hosting them over on wp.org is that images uploaded to GitHub are blocked from appearing on the mirror over at https://wordpress.org/gutenberg/handbook/ 😞

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we've had a couple conversations about this 😞 I think there's also a worry that the images will bloat the repo. Would love to find a more sustainable solution!

Copy link
Member

Choose a reason for hiding this comment

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

Right, it makes a lot of sense. I think there still are some images referenced. I remember seeing @nosolosw using GitHub hosted image. I will have to double check whether it works.

Copy link
Member

Choose a reason for hiding this comment

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

It looks like it is possible. See this page:
https://wordpress.org/gutenberg/handbook/designers-developers/developers/tutorials/plugin-sidebar-0/plugin-sidebar-1-up-and-running/

However I agree that it might negatively impact the download size of repository. It is also not clear how URL should look like. I see something like this in the source:
https://raw.githubusercontent.com/WordPress/gutenberg/master/docs/designers-developers/assets/sidebar-up-and-running.png

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I've asked about that a few times. I think the tutorials I wrote all use the Github raw CDN because that was what I had access to and what other doc folks were doing. It's a bit tricky because you have to use the image URL for when the image is deployed to master (so you don't see it during reviews). 🤷‍♂️

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

This should be reviewed also by a designer but from my perspective this looks great 👍

@gziolo gziolo added this to the Documentation & Handbook milestone Jan 22, 2019
Copy link
Contributor

@kjellr kjellr left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks, Mel.

@melchoyce melchoyce merged commit 6f6181e into WordPress:master Jan 22, 2019
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
Adding documentation to describe the use and functionality of the DropdownMenu component.

Thanks @sarahmonster and @jasmussen for drafting this.
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
Adding documentation to describe the use and functionality of the DropdownMenu component.

Thanks @sarahmonster and @jasmussen for drafting this.

Dropdown Menu is a React component to render an expandable menu of buttons. It is similar in purpose to a `<select>` element, with the distinction that it does not maintain a value. Instead, each option behaves as an action button.
The DropdownMenu displays a list of actions (each contained in a MenuItem, MenuItemsChoice, or MenuGroup) in a compact way. It appears in a Popover after the user has interacted with an element (a button or icon) or when they perform a specific action.
Copy link
Member

Choose a reason for hiding this comment

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

While it probably should use MenuItem, MenuItemsChoice, and MenuGroup components, it doesn't currently, nor did at the time of this pull request. This confused me for a moment in my debugging of #14754 (comment) .

Right now it's only rendering its own IconButton for each control.

I'll see if in the course of resolving #14754 that it could be updated to at least render MenuItem. I'm not really sure what combination of controls prop for this component would be expected to result in MenuGroup or MenuItemsChoice being rendered, however.

The way it's implemented currently, it does support grouping of controls, but notably lacks any way of expressing a label to use for a MenuGroup, so I don't know that it would be an appropriate use of that component.

<DropdownMenu
	controls={ [
		[
			{
				title: 'Group 1, Control 1',
				icon: 'arrow-up-alt',
			},
			{
				title: 'Group 1, Control 2',
				icon: 'arrow-up-alt',
			},
		],
		[
			{
				title: 'Group 2, Control 1',
				icon: 'arrow-up-alt',
			},
			{
				title: 'Group 2, Control 2',
				icon: 'arrow-up-alt',
			},
		]
	] }
/>

From these other related component documentations, I also note the phrasing "are intended to be used in a DropDownMenu", which may be misleading in that they're used by DropdownMenu, but the developer doesn't really have the choice of providing DropdownMenu with any of these components.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Developer Documentation Documentation for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants