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

Improve node selector details #1884

Merged
merged 12 commits into from
Jan 10, 2024

Conversation

MohamedElmdary
Copy link
Member

@MohamedElmdary MohamedElmdary commented Jan 4, 2024

Description

Improve node selector details

Changes

  • redesign node card
  • write a selectable table (custom component) to work as VSelect

Related Issues

SS

  1. Initial Loading (loading while there is no loaded nodes yet)
    image

  2. After first loading pick a valid node and scroll down to that node (to it's center) smoothly
    image

  3. Show load more nodes (if there is more nodes to load)
    image

  4. Ask to reload node if data changed
    image

  5. Show error if the provided filters is not valid
    image

  6. Show message while validating
    image

  7. Show loading linear line (if loading and there is nodes already there)
    image

  8. Got hint as before
    image

  9. Has hint to notify user that cpu might be more than 100% (usage)
    image

  10. some more tooltips for more details
    image
    image

  11. Deploy button disabled
    image
    image

Checklist

  • Tests included
  • Build pass
  • Documentation
  • Code format and docstrings
  • Screenshots/Video attached (needed for UI changes)

- feat: add NodeDetailsCard showing more details in node selector
- refactor: use node card in node selector
- refactor: turn node selector into VSelect instead of VAutocomplete
- style: turn VAlert into elevated variant instead of tonal
@AlaaElattar
Copy link
Contributor

i think the separator between cards should be stronger to be clear. what do u think ?

@MohamedElmdary
Copy link
Member Author

i think the separator between cards should be stronger to be clear. what do u think ?

VDivider style doesn't match with outer border so I added a border bottom instead

Copy link
Contributor

@xmonader xmonader left a comment

Choose a reason for hiding this comment

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

Let's not show a serial if it's default string

Copy link
Contributor

@AhmedHanafy725 AhmedHanafy725 left a comment

Choose a reason for hiding this comment

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

what does it mean the shared chip on the farm? I prefer to have farm name instead.

Copy link
Contributor

@mohamedamer453 mohamedamer453 left a comment

Choose a reason for hiding this comment

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

  • After the nodes are loaded it randomly scrolls to one of the nodes, i think it should load the nodes and leave it up to the user to select the desired node.
Screencast.from.08-01-24.15.54.16.webm
  • I believe as it was agreed upon before that the user doesn't need to know anything about the term storage pools and it would be better if the message was updated to something else like "checking if the deployment will fit in the node's disks" or something similar.

    image

  • IMO it would look better if the node id was written in the same way as the farm id. e.g. "Node ID: 11" instead of using brackets.

    image

  • In the manual filter, when the dedicated node filter is enabled and a shared node id is added it should mention that this node is shared in the error message instead of mentioning that this node is not rented.

    image

  • On mainnet, some nodes appeared without flag. e.g. node 2336

image

even though the node has a country in its details from the gridproxy

image

  • After Loading more nodes the storage pools check is done even when the same node is selected.

    Screencast.from.08-01-24.16.40.48.webm

@MohamedElmdary
Copy link
Member Author

what does it mean the shared chip on the farm? I prefer to have farm name instead.

For farmName we need extra request

@MohamedElmdary
Copy link
Member Author

  • After the nodes are loaded it randomly scrolls to one of the nodes, i think it should load the nodes and leave it up to the user to select the desired node.

It was a requested feature to improve ux

- fix: Temp fix for *czechia* country by adding if condition for it
- chore: open issue for that temp fix on tfchain_graphql#148
- style: Change how we show Node ID title
- typo: Update messages to be more user friendly
@mohamedamer453
Copy link
Contributor

  • After Loading more nodes the storage pools check is done even when the same node is selected.

This issue is still present

@MohamedElmdary
Copy link
Member Author

  • After Loading more nodes the storage pools check is done even when the same node is selected.

This issue is still present

It's verify that node still valid

"
/>
</div>
<!-- <div class="border-b" :style="{ borderBottomWidth: '2px !important' }" /> -->
Copy link
Contributor

Choose a reason for hiding this comment

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

remove the commented line

@xmonader xmonader merged commit aac2542 into development Jan 10, 2024
3 checks passed
@xmonader xmonader deleted the development_improve_node_selector_details branch January 10, 2024 08:36
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.

6 participants