Skip to content

♻️ change call-service row to button row#4744

Merged
bramkragten merged 11 commits intohome-assistant:devfrom
iantrich:new-button-row
Apr 1, 2020
Merged

♻️ change call-service row to button row#4744
bramkragten merged 11 commits intohome-assistant:devfrom
iantrich:new-button-row

Conversation

@iantrich
Copy link
Copy Markdown
Member

@iantrich iantrich commented Feb 4, 2020 •
edited
Loading

Breaking change

Proposed change

Rename call-service special row type to button. Will allow us to better align between row/card/element. Non-breaking change as call-service row now just extends button row

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

image

type: entities
entities:
  - entity: light.bed_light
  - type: button
    service: light.toggle
    name: Light Toggle Button
    action_name: Button
    service_data:
      entity_id: light.bed_light
  - type: call-service
    service: light.toggle
    name: Light Toggle Call-Service
    action_name: Call-Service
    service_data:
      entity_id: light.bed_light

Additional information

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:

@iantrich iantrich requested a review from bramkragten February 4, 2020 04:40
@iantrich iantrich self-assigned this Feb 4, 2020
@bramkragten
Copy link
Copy Markdown
Member

bramkragten commented Feb 4, 2020

Nice, we should have scene and script row extend this as well.

bramkragten
bramkragten reviewed Feb 4, 2020
View reviewed changes
Comment thread src/panels/lovelace/entity-rows/types.ts Outdated
bramkragten
bramkragten reviewed Feb 17, 2020
View reviewed changes
Comment thread src/panels/lovelace/special-rows/hui-button-row.ts Outdated
bramkragten
bramkragten reviewed Feb 17, 2020
View reviewed changes
Comment thread src/panels/lovelace/entity-rows/types.ts
bramkragten
bramkragten reviewed Feb 17, 2020
View reviewed changes
Comment thread src/panels/lovelace/entity-rows/types.ts
@iantrich iantrich requested a review from bramkragten March 29, 2020 21:53
@iantrich iantrich mentioned this pull request Mar 29, 2020
Merged
7 tasks
bramkragten
bramkragten reviewed Mar 31, 2020
View reviewed changes
Comment thread src/panels/lovelace/special-rows/hui-call-service-row.ts Outdated
zsarnett
zsarnett reviewed Apr 1, 2020
View reviewed changes
Comment thread src/panels/lovelace/special-rows/hui-button-row.ts Outdated
bramkragten
bramkragten reviewed Apr 1, 2020
View reviewed changes
Comment thread src/panels/lovelace/special-rows/hui-call-service-row.ts Outdated
@property() private _config?: CallServiceConfig;
export class HuiCallServiceRow extends HuiButtonRow {
public setConfig(config: ButtonRowConfig): void {
const callServiceConfig: any = config;
Copy link
Copy Markdown
Member

@bramkragten bramkragten Apr 1, 2020

Choose a reason for hiding this comment

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

Why should keep the interface for CallServiceConfig

bramkragten
bramkragten reviewed Apr 1, 2020
View reviewed changes
Comment thread src/panels/lovelace/special-rows/hui-call-service-row.ts Outdated

@property() private _config?: CallServiceConfig;
export class HuiCallServiceRow extends HuiButtonRow {
public setConfig(config: ButtonRowConfig): void {
Copy link
Copy Markdown
Member

@bramkragten bramkragten Apr 1, 2020

Choose a reason for hiding this comment

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

Suggested change
public setConfig(config: ButtonRowConfig): void {
public setConfig(config: CallServiceConfig): void {

Copy link
Copy Markdown
Member Author

@iantrich iantrich Apr 1, 2020

Choose a reason for hiding this comment

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

Can't change the signature of setConfig

Copy link
Copy Markdown
Member

@bramkragten bramkragten Apr 1, 2020

Choose a reason for hiding this comment

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

Yeah already "fixed" it...

Copy link
Copy Markdown
Contributor

@zsarnett zsarnett Apr 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I think the best way would be

const buttonRowConfig: ButtonRowConfig = {...config, type: 'button'};

super.setConfig(buttonRowConfig);

Similar to how the Sensor card does for entity card extending

iantrich
iantrich commented Apr 1, 2020
View reviewed changes
Comment thread src/panels/lovelace/entity-rows/types.ts Outdated
| WeblinkConfig
| ButtonsRowConfig
| ConditionalRowConfig
| ButtonRowConfig
Copy link
Copy Markdown
Member Author

@iantrich iantrich Apr 1, 2020

Choose a reason for hiding this comment

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

why did you remove this?

Copy link
Copy Markdown
Member

@bramkragten bramkragten Apr 1, 2020

Choose a reason for hiding this comment

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

Freaking merge conflict

@bramkragten bramkragten merged commit 26d27f8 into home-assistant:dev Apr 1, 2020
@bramkragten bramkragten mentioned this pull request Apr 1, 2020
Merged
@lock lock Bot locked and limited conversation to collaborators Apr 2, 2020
@iantrich iantrich deleted the new-button-row branch May 2, 2020 19:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@bramkragten bramkragten bramkragten approved these changes

@zsarnett zsarnett zsarnett left review comments

Assignees

@iantrich iantrich

Labels

cla-signed Documentation Added

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@iantrich @bramkragten @zsarnett @homeassistant