Skip to content

Add an 'Add a Class' entry to the edit menu #111

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

Merged
merged 1 commit into from
Mar 9, 2016

Conversation

bgw
Copy link
Contributor

@bgw bgw commented Mar 9, 2016

The data browser 'Edit' menu has related options like 'Add a Row' or 'Delete this class', so 'Add a class' is a natural addition.

The data browser 'Edit' menu has related options like 'Add a Row' or
'Delete this class', so 'Add a class' is a natural addition.
@facebook-github-bot
Copy link

By analyzing the blame information on this pull request, we identified @drew-gross and @durunvo to be potential reviewers.

@peterdotjs
Copy link
Contributor

nice! one nit - could we add it at the bottom of the menu with a separator?

<Separator />
<MenuItem text='Add a class' onClick={onAddClass} />

My reasoning is that other actions are specific to the class itself. And this is for creating a new class.

@drew-gross
Copy link
Contributor

I actually like it where it is - all the "add" operations are grouped and all the "delete" operations are grouped.

@pie6k
Copy link

pie6k commented Mar 9, 2016

There is a lot of space there, My skills don't allow me to make PR, but from my UX experience I see it like it's begging to make some icons there instead of dropdown to reduce task from 2 clicks to 1 click and to make you instantly see what you can do.

@drew-gross
Copy link
Contributor

@adampietrasiak agreed - I would love to see it responsively move items from the toolbar to the dropdown as the browser window shrinks. That is a bigger effort though :) A medium amount of effort would be to have all the items always in the dropdown, and have only the toolbar be responsive.

@peterdotjs
Copy link
Contributor

@drew-gross it is nice that they are grouped together but odd that it is isn't specific to this class like the others. I don't think it's really worth arguing about. I'll go ahead and merge.

@adampietrasiak we had some plans to move the actions out of the dropdown towards the left side of the header which would make for better accessibility but we never got to it.

peterdotjs added a commit that referenced this pull request Mar 9, 2016
Add an 'Add a Class' entry to the edit menu
@peterdotjs peterdotjs merged commit 7c6db0c into parse-community:master Mar 9, 2016
@bgw
Copy link
Contributor Author

bgw commented Mar 9, 2016

I'd be interested in taking a stab at implementing some sort of responsive toolbar, if people think it's worth the engineering effort.

douglasmuraoka pushed a commit that referenced this pull request Nov 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants