Menu bar

[fatih-erikli]

Fixes #1027

[justvanrossum]

Great start, thanks!

I haven't reviewed the code, but playing with it I notice two things:

• the font size for the menu titles is too small, and should probably match those tof the menu-panel items
• hovering the menu title only works once the menu bar has focus, or at least has been clicked once. I think it should always hover.

[justvanrossum]

Ok, trying again, and comparing with google docs: the behavior is fine, except the menu title should also highlight when hovering over.

[fatih-erikli]

hovering the menu title only works once the menu bar has focus, or at least has been clicked once. I think it should always hover.

Ok. I copied the behavior of top bar in macos.

[fatih-erikli]

I did force push to rebase.

[justvanrossum]

This looks good, but I would like the items list to not be static. I think the items field should be a function instead, and be called getItems. It is then also important that this getItems function is only called when the menu is displayed, so that it really gets populated on-demand.

The same goes for sub-menu: a sub menu should be populated only right before it is displayed.

[justvanrossum]

(Also, please rebase again, also for sub-menu, although I'm a bit confused how that works, since the latter is a PR on menu-bar.)

[fatih-erikli]

Not ready for review yet.

[justvanrossum]

For now, let's do three links under "Help":

• Homepage: https://fontra.xyz/
• Documentation: https://gferreira.github.io/fontra-docs/
• GitHub: https://github.com/googlefonts/fontra

[justvanrossum]

Thanks for the refinements!

Further feedback:

• The "Edit" menu doesn't work when a glyph is in edit mode
• Glyph menu: add "Delete source menu...", "Edit local axes..."
• Select previous/next source and Find glyphs that use ... should go under the Glyph menu
• All menus that show a dialog (such as "Add source") should have titles that end with "..."
• The Add source and Delete source menu's need dymanic enable/disable

[justvanrossum]

Select previous/next source and Find glyphs that use ... should go under the Glyph menu

Or would they be better under "View"?

[fatih-erikli]

I noticed one little bug, Esc key closes the open menu panel, but the panels keeps opening when hovering on other menu items.

[justvanrossum]

I noticed one little bug, Esc key closes the open menu panel, but the panels keeps opening when hovering on other menu items.

Can you fix it?

[justvanrossum]

Select previous/next source and Find glyphs that use ... should go under the Glyph menu

Like I commented later, I think they actually should go under "View".

[justvanrossum]

Can we simplify the constructor? I'm thinking all but the first arguments should go into an options object:

  constructor(menuItems, options) {

Since many of these are optional this should simplify things. Also, it makes it easier to add more if we need.

[fatih-erikli]

Yes, you're right.

[justvanrossum]

Let's merge this! Looking good to me.