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

bug(menu): mat-divider inside a menu missing margin #19248

Open
thw0rted opened this issue May 4, 2020 · 7 comments
Open

bug(menu): mat-divider inside a menu missing margin #19248

thw0rted opened this issue May 4, 2020 · 7 comments
Labels
area: material/divider area: material/menu material spec Issue related to the Material Design spec https://material.io/design/ P4 A relatively minor issue that is not relevant to core functions

Comments

@thw0rted
Copy link

thw0rted commented May 4, 2020

Reproduction

See Stackblitz

Expected Behavior

The relevant spec section mandates 8px above and below dividers in a menu. See the example headed "Desktop".

Actual Behavior

The divider has no margin above or below. Hover over the items adjacent to it and you can see that their highlight region touches the divider.

Environment

  • Angular: 9.1.4
  • CDK/Material: 9.2.1
  • Browser(s): Irrelevant
  • Operating System (e.g. Windows, macOS, Ubuntu): Irrelevant
@thw0rted thw0rted added the needs triage This issue needs to be triaged by the team label May 4, 2020
@jelbourn jelbourn added material spec Issue related to the Material Design spec https://material.io/design/ P4 A relatively minor issue that is not relevant to core functions and removed needs triage This issue needs to be triaged by the team labels May 29, 2020
@jelbourn
Copy link
Member

@crisbeto I checked to see if this was fixed for free in MDC, but it looks like we actually use mat-divider in this context, which doesn't have the .mdc-list-divider class that would give it right right margin. We should think about either making the divider have this inside of a menu, or adding a separate menu-item-separator.

@devversion
Copy link
Member

Linking to #19248 so that we keep track of it as part of finishing up the MDC-based menu.

@thw0rted
Copy link
Author

Ping so the lockbot doesn't eat this...

@jelbourn
Copy link
Member

@thw0rted only closed issues get auto-locked. I wouldn't allow a bot to auto-close issues.

@thw0rted
Copy link
Author

Ah, that explains it. I actually wrote up an issue this morning to ask the team to document the lock-bot behavior (it's not in the FAQ or CONTRIBUTING.md) because on some other repos, there's a "stale bot" that does close open issues that have no recent activity, but it issues a warning first.

Also, some of the repos I follow tend to close issues with a comment like "we're not planning to do this now, but we should discuss further and maybe we'll change our minds". I don't think that happens here very often, but in that kind of situation, locking closed issues would mean having to open a new ticket to continue the conversation, which seems like a bad outcome.

Anyway, it's good to know that the lock-bot won't eat open issues; I'll try to remember that that's how the policy works here. (And maybe I'll file the meta-issue about documenting the behavior after all...)

@jelbourn
Copy link
Member

jelbourn commented Jun 29, 2020

@thw0rted the lock-bot actually links to the docs in the comment it leaves:
https://github.com/angular/angular/blob/67d80f/docs/GITHUB_PROCESS.md#conversation-locking

@thw0rted
Copy link
Author

There is not an emote reaction for 🥺 so:

😳

(Sorry, and thanks)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: material/divider area: material/menu material spec Issue related to the Material Design spec https://material.io/design/ P4 A relatively minor issue that is not relevant to core functions
Projects
None yet
Development

No branches or pull requests

3 participants