Skip to content

Allow custom 2nd line in state-info#253

Merged
balloob merged 5 commits into
home-assistant:masterfrom
andrey-git:state_info
Apr 9, 2017
Merged

Allow custom 2nd line in state-info#253
balloob merged 5 commits into
home-assistant:masterfrom
andrey-git:state_info

Conversation

@andrey-git
Copy link
Copy Markdown
Contributor

Allow custom 2nd line in state-info.

To be used in custom UI.

Usage:

<state-info secondary-line><span>Text</span></state-info>

I didn't find a way to change line-height based on existing-vs-missing slot so I used an extra optional attribute for it.

@mention-bot
Copy link
Copy Markdown

@andrey-git, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob to be a potential reviewer.

@balloob
Copy link
Copy Markdown
Member

balloob commented Apr 3, 2017

I don't like this. Although I am fine with supporting custom UI, I don't want to complicate our UI so it can be used in custom UI.

@andrey-git
Copy link
Copy Markdown
Contributor Author

I could fork the state-info for custom UI, but I think this change is well-contained and might be beneficial for the core UI in the future.

The host([secondary-line]) part could be replaced with a property (like inDialog) or a mixin.

The slot could be unconditional and the caller would have to worry about inDialog compatibility.

I could remove the ::slotted(*) part and style the slot from outside.

It would even be possible to just add the <slot> element and control line-height by overriding --paper-font-common-nowrap

@balloob
Copy link
Copy Markdown
Member

balloob commented Apr 3, 2017

Overriding some mixin so you can use a component sounds like a bad API. The APi itself with in-dialog is already pretty bad. It should have been something as include-last-changed or something.

My concern is that since the default UI is not using this, it will not be tested when we make changes.

@andrey-git
Copy link
Copy Markdown
Contributor Author

Here you go - polymer tests setup :)
#255

Tests for this specific element will be written after that PR is submitted :)

@balloob
Copy link
Copy Markdown
Member

balloob commented Apr 4, 2017

okay, you win 😜 Test foundation merged.

@bdurrer
Copy link
Copy Markdown
Contributor

bdurrer commented Apr 4, 2017

Haha what a good strategy, tests as candy :-D

@andrey-git
Copy link
Copy Markdown
Contributor Author

After #256 is merged I'll add tests to this PR

@balloob
Copy link
Copy Markdown
Member

balloob commented Apr 8, 2017

Merged the test PR.

@andrey-git
Copy link
Copy Markdown
Contributor Author

andrey-git commented Apr 9, 2017

Tests added. Travis run with extra browsers: https://travis-ci.org/andrey-git/home-assistant-polymer/jobs/220284040

@balloob balloob merged commit 1530c0c into home-assistant:master Apr 9, 2017
@andrey-git andrey-git deleted the state_info branch April 9, 2017 18:23
tkdrob pushed a commit to tkdrob/frontend that referenced this pull request Apr 20, 2021
In Docusaurus V2 alpha 58 which should drop any moment via dependabot, there is a breaking change that a page will not be built for /docs which we currently have as the getting started guide. This PR just puts in a quick redirect to cover that. Also fixes some errors in the last ones (my bad)
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 8, 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.

5 participants