Skip to content

docs(material/theming): rewrite elevation guide #22467

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
Apr 21, 2021

Conversation

jelbourn
Copy link
Member

This change updates the elevation guide to be more clear, concise, and
complete. The changes include:

  • Drop content about the general concept of elevation, deferring that to
    the Material Design spec.
  • Document the overridable-elevation mixin
  • Mention that the elevation CSS classes are emitted by core-theme.
  • Switch to @use

@jelbourn jelbourn added P2 The issue is important to a large percentage of users, with a workaround docs This issue is related to documentation merge safe target: major This PR is targeted for the next major release area: theming labels Apr 13, 2021
@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Apr 13, 2021
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM

`$zValue` is number from 0 to 24, representing the semantic elevation of the element, that controls
the intensity of the box-shadow. You can use the `$color` and `$opacity` parameters to further
customize the shadow appearance. Because an elevation shadow consists of multiple shadow components
of varying opacities, the `$opacity` argument of the mixin acts as a factor by which to scale these
Copy link
Member

Choose a reason for hiding this comment

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

I think that we might want to not mention the opacity parameter since the MDC mixins don't support it. They have something like an "opacity boost" where it adds some opacity to the base.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed

@jelbourn jelbourn force-pushed the rewrite-elevation branch from c1f92cc to 2f5cf9b Compare April 13, 2021 18:30
Copy link
Contributor

@twerske twerske left a comment

Choose a reason for hiding this comment

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

lgtm

### Overridable elevation

When authoring a component, you may want to specify a default elevation that the component consumer
can override. You can accomplish this by using the `overridable-elevation` Sass mixin. This behaves
Copy link
Contributor

Choose a reason for hiding this comment

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

[stretch] adding an example of the override to show this overridable usecase?

I'm not strongly opinionated on this

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll pass on this since I think it's kind of niche and also laziness.

@jelbourn jelbourn added target: rc This PR is targeted for the next release-candidate action: merge The PR is ready for merge by the caretaker and removed target: major This PR is targeted for the next major release labels Apr 14, 2021
Copy link

@TeriGlover TeriGlover left a comment

Choose a reason for hiding this comment

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

Editing review completed; left a few suggestions.

@TeriGlover
Copy link

TeriGlover commented Apr 19, 2021 via email

This change updates the elevation guide to be more clear, concise, and
complete. The changes include:
* Drop content about the general concept of elevation, deferring that to
  the Material Design spec.
* Document the `overridable-elevation` mixin
* Mention that the elevation CSS classes are emitted by `core-theme`.
* Switch to `@use`
@jelbourn jelbourn force-pushed the rewrite-elevation branch from 2f5cf9b to d13274d Compare April 19, 2021 23:33
@wagnermaciel wagnermaciel merged commit 3e4e6d0 into angular:master Apr 21, 2021
wagnermaciel pushed a commit that referenced this pull request Apr 21, 2021
This change updates the elevation guide to be more clear, concise, and
complete. The changes include:
* Drop content about the general concept of elevation, deferring that to
  the Material Design spec.
* Document the `overridable-elevation` mixin
* Mention that the elevation CSS classes are emitted by `core-theme`.
* Switch to `@use`

(cherry picked from commit 3e4e6d0)
@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 May 22, 2021
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 area: theming cla: yes PR author has agreed to Google's Contributor License Agreement docs This issue is related to documentation P2 The issue is important to a large percentage of users, with a workaround target: rc This PR is targeted for the next release-candidate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants