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

Proposal: New resources for styling menus consistently #4594

Open
ghost opened this issue Mar 20, 2021 · 2 comments
Open

Proposal: New resources for styling menus consistently #4594

ghost opened this issue Mar 20, 2021 · 2 comments
Labels
area-Resources feature proposal New feature proposal team-Controls Issue for the Controls team

Comments

@ghost
Copy link

ghost commented Mar 20, 2021

Proposal: New resources for styling menus consistently

Summary

I propose adding a couple new common resources to easily set the same padding, margin and item height among various menus.

Rationale

  • There are many menus that are very similar in functionality and looks, but their design is implemented manually each time, and differences occur frequently.
  • ControlCornerRadius and OverlayCornerRadius are used in almost all controls with rounded corners so that the values can be changed once for all of them.
  • Users would be able to easily style all menus at once instead of setting values for each of them.

Scope

Capability Priority
New resources are added Must
Existing resources continue working Must
Resources are added to controls as needed Should

Important Notes

  • Horizontal menus (e.g. CommandBar, NavigationView in top mode, MenuBar)
    HorizontalMenuPadding
    HorizontalMenuItemMargin
    HorizontalMenuItemHeight

  • Vertical menus (all flyouts with a list of buttons)
    VerticalMenuPadding
    VerticalMenuItemMargin
    VerticalMenuItemHeight

Controls can be updated to use these resources. All existing lightweight resources should continue to work.

Where they can be used:

  • MenuItemMargin, MenuItemHeight (for items): MenuFlyoutItem, MenuFlyoutSubItem, RadioMenuFlyoutItem, ToggleMenuFlyoutItem, NavigationViewItem, ComboBoxItem, AppBarButton, AppBarToggleButton
  • MenuPadding (for containers): CommandBar, CommandBar.SecondaryCommands, CommandBarFlyout, MenuFlyout, NavigationView, ComboBox

Here is a working example of horizontal and vertical menus using those resources:
commandbar

This proposal can solve all inconsistencies in #4383.

Most controls would only need to change explicit values to those resources. Some controls might need very minor changes.

Extra

CommandBar has resources to set an inner margin, which allows the item to be hovered from outside the visual. This would continue to work with this proposal, but it could be extended to the other menus in the future.

Open questions

  • What would be better names for the resources?
  • What should the default values be? Right now it seems that vertical menus are supposed to have 2px padding and 2px item margin.

Additional notes

If this is approved, I'd like to work on this.

@ghost ghost added the feature proposal New feature proposal label Mar 20, 2021
@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Mar 20, 2021
@StephenLPeters
Copy link
Contributor

I would love to increase the number of levels we do resource redirection at, however our system doens't really support this very well. When a resource lookup occurs in the framework, the systems starts looking for a resource with the key "foo" when it finds a resource somewhere in the tree that redirects to a different resource named "bar" the systems continues its lookup from the spot that it found "foo". This means that if "bar" was defined ad a narrower scope than we found "foo" at, the system would miss the "bar" declaration. This really prevents us from having a robust resourcing story, like what you suggesting would help us with. However it is much more performant.

@StephenLPeters
Copy link
Contributor

So this proposal is dependent on #2913

@StephenLPeters StephenLPeters added team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Mar 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Resources feature proposal New feature proposal team-Controls Issue for the Controls team
Projects
None yet
Development

No branches or pull requests

1 participant