Skip to content

Add help link on automations picker and updated links for scripts and scenes#7129

Merged
zsarnett merged 2 commits intohome-assistant:devfrom
Villhellm:help-links
Oct 11, 2020
Merged

Add help link on automations picker and updated links for scripts and scenes#7129
zsarnett merged 2 commits intohome-assistant:devfrom
Villhellm:help-links

Conversation

@Villhellm
Copy link
Copy Markdown
Contributor

Breaking change

Proposed change

Add a help link to the automation picker panel. Scripts and scenes have this link already. Just trying to make the panel a little more consistent.

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:

Comment on lines +174 to +178
<ha-icon-button
slot="toolbar-icon"
icon="hass:help-circle"
@click=${this._showHelp}
></ha-icon-button>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
<ha-icon-button
slot="toolbar-icon"
icon="hass:help-circle"
@click=${this._showHelp}
></ha-icon-button>
<mwc-icon-button @click=${this._showHelp}>
<ha-svg-button
slot="toolbar-icon"
.path=${mdiHelpCircle}
></ha-svg-button>
</mwc-icon-button>

This formatting is probably not perfect

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I pulled that code from both the scripts picker and the scenes picker. Are you saying both need to be updated?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes. We recently moved to use this method with the new MDI/JS library

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm happy to change all of them, I just want to confirm that's what needs to be done

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmm. That change makes the icons disappear. I'm getting an error in the console
DevTools failed to load SourceMap: Could not parse content for http://10.1.1.4:8124/config/scene/mdc.circular-progress.min.css.map: Unexpected token < in JSON at position 0

Copy link
Copy Markdown
Contributor

@zsarnett zsarnett Sep 25, 2020

Choose a reason for hiding this comment

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

That error has nothing to do with that, it will show anytime a circular progress is used. Make sure to import the correct elements

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry :/ I can't get the icon to show up again. I think I have all the imports it needs and I'm not seeing any errors. I'll try again tomorrow I suppose

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oh
I know what it is

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I fucked the comment up
its supposed to ha-svg-icon

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah yup, that was part of it. Thanks for catching that! The other thing appears to be slot needing to be in mwc-icon-button instead of inside ha-svg-icon

@zsarnett
Copy link
Copy Markdown
Contributor

Got a screen shot?

@Villhellm
Copy link
Copy Markdown
Contributor Author

Screen Shot 2020-09-24 at 8 30 52 PM

@Villhellm
Copy link
Copy Markdown
Contributor Author

I'll rebase in the morning and get some screenshots to verify the changes

@Villhellm Villhellm changed the title Add help link on automations picker Add help link on automations picker and updated links for scripts and scenes Sep 25, 2020
@Villhellm
Copy link
Copy Markdown
Contributor Author

Screen Shot 2020-09-24 at 9 55 12 PM

import { DataTableColumnContainer } from "../../../components/data-table/ha-data-table";
import { showAlertDialog } from "../../../dialogs/generic/show-dialog-box";
import "../../../components/entity/ha-entity-toggle";
import "../../../components/ha-icon-button";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we still use this?

@zsarnett zsarnett merged commit cc0515c into home-assistant:dev Oct 11, 2020
@bramkragten bramkragten mentioned this pull request Oct 21, 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.

4 participants