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

Added note about more integrations to bottom of Set up new integration dialog #3992

Merged
merged 2 commits into from Oct 13, 2019

Conversation

springstan
Copy link
Member

Issues affected: #3951

Description:

  • added a <p> to the bottom of the custom html element step-flow-pick-handler
  • including two <span> elements containing the note about more integrations
  • added three key-value pairs to src/translations/en.json

Result:
grafik

@@ -79,6 +80,22 @@ class StepFlowPickHandler extends LitElement {
`
)}
</div>
<p style=${styleMap({ textAlign: `center` })}>
Copy link
Member

Choose a reason for hiding this comment

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

Don't use a styleMap for static styles, they are for dynamic styles. Just add this styles to the static get styles()

<span>
${this.hass.localize(
"ui.panel.config.integrations.note_about_website_reference"
)}<a href=${websiteLink}
Copy link
Member

Choose a reason for hiding this comment

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

Just place the link inline instead of in a variable.

Copy link
Member

@bramkragten bramkragten left a comment

Choose a reason for hiding this comment

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

Thanks! Some small changes and it is good to go! 🦁

@@ -79,6 +80,22 @@ class StepFlowPickHandler extends LitElement {
`
)}
</div>
<p style=${styleMap({ textAlign: `center` })}>
<span style=${styleMap({ display: `block` })}>
Copy link
Member

Choose a reason for hiding this comment

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

A span with display: block, that's a p right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah you are right, I probably made it more difficult than it needs to be. However, if I put everything into one p it will not automatically handle the line break after the first sentence. Which does not look very pretty:
grafik
Any suggestions on how to deal with this problem?

Copy link
Member

Choose a reason for hiding this comment

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

Use a line break <br>

Copy link
Member Author

Choose a reason for hiding this comment

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

Thought that using them was not optimal but sure I can do that.
Hopefully a9ff740 addresses all your recommended changes :)

"ui.panel.config.integrations.note_about_integrations"
)}
</span>
<span>
Copy link
Member

Choose a reason for hiding this comment

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

Why another span? This can all go straight in the p

Copy link
Member

@bramkragten bramkragten left a comment

Choose a reason for hiding this comment

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

Thanks! 🦁

@bramkragten bramkragten merged commit b94da1b into home-assistant:dev Oct 13, 2019
@github-actions github-actions bot locked and limited conversation to collaborators Jul 6, 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

5 participants