changes to allow more menu item class flexibility

jonsutherland commented

2017-02-07 21:32:40 +01:00

(Migrated from github.com)

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 append multiple classes to a MenuItem easily. Add a way to provide the wrapping Prosemirror-menuitem span a class with 'menuItemClass' spec property.

marijnh commented

2017-02-07 22:01:13 +01:00

(Migrated from github.com)

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?

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 commented

2017-02-07 22:30:09 +01:00

(Migrated from github.com)

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.

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 commented

2017-02-07 23:04:45 +01:00

(Migrated from github.com)

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.

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.

marijnh commented

2017-02-08 12:20:09 +01:00

(Migrated from github.com)

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.

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 commented

2017-02-08 12:46:49 +01:00

(Migrated from github.com)

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.

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Rows
Columns

changes to allow more menu item class flexibility #4

Pull request closed