Skip to content

docs(cdk-experimental/menu): add examples to menu documentation #20482

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

Merged
merged 1 commit into from
Sep 22, 2020

Conversation

andy9775
Copy link
Contributor

@andy9775 andy9775 commented Sep 2, 2020

I'm not 100% sure if I connected the examples with menu.md correctly. PTAL.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Sep 2, 2020
@@ -0,0 +1,33 @@
.cdk-menu {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the examples, we try to prefix all of the styles with .example- so it's clear where they're coming from

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those are the classes that the cdk directives add.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we can either add additional class in the example, or put all of these as child selectors, e.g.

.example-menu { ... }
.example-menuitem { ...}

/** OR **/
.example-menu .cdk-menu {
  ...
}

.example-menu .cdk-menuitem {
  ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, ok. I see what you were saying now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jelbourn Done


.cdk-menu hr {
width: 100%;
color: #0000001f;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the alpha notation here works in older browsers

@andy9775
Copy link
Contributor Author

andy9775 commented Sep 3, 2020

@jelbourn feedback should be addressed.

Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

(I was on vacation for a bit)

@jelbourn jelbourn added docs This issue is related to documentation action: merge The PR is ready for merge by the caretaker merge safe P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent labels Sep 17, 2020
@wagnermaciel wagnermaciel added the target: patch This PR is targeted for the next patch release label Sep 18, 2020
@andy9775
Copy link
Contributor Author

@jelbourn no problem.

Hope your vacation was good.

@jelbourn jelbourn added target: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels Sep 22, 2020
@annieyw annieyw merged commit 469984f into angular:master Sep 22, 2020
wagnermaciel pushed a commit to wagnermaciel/components that referenced this pull request Sep 24, 2020
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Oct 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement docs This issue is related to documentation P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants