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

chore(launchpad): refactor status screen #2231

Merged
merged 5 commits into from
Oct 15, 2024

Conversation

mazzi
Copy link
Contributor

@mazzi mazzi commented Oct 14, 2024

Description

To unify patterns and to be prepared for the next feature, we use now NodeItem and a StatefulTable for the status screen as we use on the drive selection and connection popups.

With this architecture is easier to understand how to update the real time node status and changing rendering modes per node. We update stats and spinners on Tick.

Next feature: Call To Actions on individual nodes.

fn update_node_items(&mut self) -> Result<()> {
// Iterate over existing node services and update their corresponding NodeItem
if let Some(ref mut items) = self.items {
for (node_item, item) in self.node_services.iter().zip(&mut items.items) {
Copy link
Member

Choose a reason for hiding this comment

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

Would this cause an issue if node_services.len() > items.len()? The items field is updated only during // If items is None, create a new list (fallback) part of the code. This would mean that if we were to add a new node, just the node_services would be updated and not items. Then the zip() would ignore these new nodes.

Copy link
Member

Choose a reason for hiding this comment

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

The solution is to keep node_services and items in sync.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We will not delete nodes just now. That hopefully will come in the next PR when we manage nodes individually.
Thanks for your feedback @RolandSherwin !

@mazzi mazzi requested a review from RolandSherwin October 14, 2024 16:54
@mazzi mazzi added this pull request to the merge queue Oct 15, 2024
Merged via the queue into maidsafe:main with commit b1a0b00 Oct 15, 2024
24 checks passed
@mazzi mazzi deleted the chore_refactor_status_screen branch October 15, 2024 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants