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

Entity dialogs: Remove paper element and align dialog header style #6370

Merged
merged 2 commits into from
Jul 13, 2020

Conversation

bramkragten
Copy link
Member

Proposed change

Align the header style on desktop more with default material dialog styles as we use throughout the app.

Replace paper-tabs and app-toolbar with LitElement versions.

image

image

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

@property({ type: Boolean }) centerTitle = false;

protected render() {
let title = html`<span class="mdc-top-app-bar__title">
Copy link
Member

Choose a reason for hiding this comment

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

Let's make this a ternary statement.

Copy link
Member

Choose a reason for hiding this comment

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

or if…else.

Copy link
Member Author

Choose a reason for hiding this comment

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

the second one uses the first one.

Copy link
Member Author

Choose a reason for hiding this comment

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

But I ended up not using it because it doesn't work well, so might as well remove it...

unsafeCSS(topAppBarStyles),
css`
.mdc-top-app-bar {
position: static;
Copy link
Member

Choose a reason for hiding this comment

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

Are we sure we want to put this in the element itself and not have the parent decide on this?

Copy link
Member

Choose a reason for hiding this comment

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

This could be used outside of dialogs, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

If you would want to use on the top scope you should use <mwc-top-app-bar>

@bramkragten bramkragten merged commit 1c73007 into dev Jul 13, 2020
@bramkragten bramkragten deleted the more-info-entity-settings-style branch July 13, 2020 17:37
@bramkragten bramkragten mentioned this pull request Jul 14, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jul 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants