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

changes to allow more menu item class flexibility #4

Closed
wants to merge 1 commit into from
Closed

changes to allow more menu item class flexibility #4

wants to merge 1 commit into from

Conversation

jonsutherland
Copy link

Add a way to append multiple classes to a MenuItem easily.

Add a way to provide the wrapping Prosemirror-menuitem span a class with 'menuItemClass' spec property.

Add a way to provide the wrapping Prosemirror-menuitem span a class.
@marijnh
Copy link
Member

marijnh commented Feb 7, 2017

I don't really like that this reaches into the menu elements, which are currently only expected to have a render method. Why do you need extra classes on the item wrappers?

@jonsutherland
Copy link
Author

Yeah I don't really like it either. But the CSS designer is asking for it. I pushed back and demanded to know why it's required. Basically they want control over spacing and margin and other things that I don't think of myself, as a programmer. They say the parent's Prosemirror-menuitem's aren't specific enough for them.

I'd be okay with coming up with a different solution where I add the classes programatically on load, but it feels a little disconnected too.

@jonsutherland
Copy link
Author

Whoops I didn't mean to close it, yet.

For instance, the type change drop down, they want to add more spacing around it and they don't like using span:nth since they items change based on the state.

@jonsutherland jonsutherland reopened this Feb 7, 2017
@marijnh
Copy link
Member

marijnh commented Feb 8, 2017

Would a margin on the inner element not have the same effect?

In general, this repository should be seen as a proof of concept or example, and, unlike the core ProseMirror modules, I'm not going to make it support all use cases. If you have precise styling requirements, consider just writing your own custom menubar.

@jonsutherland
Copy link
Author

Thanks very much for the reply. Design people still think that they need a custom class on the top parent element. I've yet to convince them otherwise. I've already forked your menu module so that's the road we'll take.

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.

3 participants