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.

Sign up for GitHub

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

Jump to bottom

Add basic nodes list & node metadata to OZW config panel #6719

Merged
merged 16 commits into from
Sep 4, 2020
Merged

Add basic nodes list & node metadata to OZW config panel #6719

merged 16 commits into from
Sep 4, 2020

Conversation

cgarwood
Copy link
Member

Proposed change

  • Adds basic nodes list to OZW config panel
  • Restructures config panel file layout to better handle additional pages

image

I'm currently having some issues I haven't been able to figure out with the ozw-network-nodes page and <ha-tabs-subpage-data-table> giving an error that ResizeObserver loop limit exceeded.

I'd also love some tips on centrally managing the navigation tags (basically anything under ozw-network-router should have the same tabs, anything under ozw-node-router should have a separate set of tabs).

I'm also not fully convinced I'm managing the routers correctly... let me know if there's a better way to optimize or handle those.

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

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.

@bramkragten bramkragten marked this pull request as draft August 27, 2020 20:04
@cgarwood cgarwood marked this pull request as ready for review August 31, 2020 16:28
@cgarwood
Copy link
Member Author

cgarwood commented Aug 31, 2020 •
edited
Loading

Added some quick content to the node details page for this first PR. Ready for final review now. I didn't do translations on the Node Management page yet as this isn't its final form

image

@cgarwood cgarwood changed the title WIP: Add basic nodes list to OZW config panel Add basic nodes list & node metadata to OZW config panel Aug 31, 2020
bramkragten
bramkragten reviewed Sep 3, 2020
View reviewed changes
src/panels/config/integrations/integration-panels/ozw/ozw-node-dashboard.ts Outdated
Comment on lines 138 to 147
this._node = await fetchOZWNodeStatus(
this.hass!,
this.ozwInstance,
this.nodeId
);
this._metadata = await fetchOZWNodeMetadata(
this.hass!,
this.ozwInstance,
this.nodeId
);
Copy link
Member

Choose a reason for hiding this comment

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

What will happen when we request a node that no longer/not exists?

Copy link
Member

Choose a reason for hiding this comment

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

So this will not error but return None?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated to catch a not_found error and show the error page. I used a separate _not_found property to prevent a quick flash of the not found page on initial load before it finishes fetching the node data initially.

@bramkragten bramkragten merged commit aa5e20d into home-assistant:dev Sep 4, 2020
@bramkragten bramkragten mentioned this pull request Sep 4, 2020
Merged
@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.
Reviewers

@zsarnett zsarnett zsarnett left review comments

@bramkragten bramkragten bramkragten approved these changes

Assignees
No one assigned
Labels
cla-signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cgarwood @bramkragten @zsarnett @homeassistant