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

Add serial lookup on peers table + permit filtering on 'All' group only #425

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

EdouardVanbelle
Copy link

@EdouardVanbelle EdouardVanbelle commented Nov 27, 2024

Please find a very small improvement in peers list:

  • permit a serial lookup on peers table
  • display the peer's serial number (to avoid. new column I added it in the OS section:
Serial Number
  • display of the serial number in the peer section

  • capability to filter peers that belong to the "All" group only, this is useful to track non assigned peers
    All group only

@EdouardVanbelle EdouardVanbelle force-pushed the add-serial-lookup branch 2 times, most recently from 4e56441 to 2c469c0 Compare November 29, 2024 21:55
@EdouardVanbelle EdouardVanbelle marked this pull request as ready for review November 29, 2024 22:01
@heisbrot
Copy link
Contributor

Hey @EdouardVanbelle,

thanks for this PR! I like the idea of filtering by unassigned peers.
However, we should look into having it in another position to keep the filters clean.

I was thinking of either in the groups filter or having a dropdown with (All Peers, Online Peers, Offline Peers, Unassigned Peers) instead of the current buttons. What do you think?

@EdouardVanbelle
Copy link
Author

EdouardVanbelle commented Dec 21, 2024

Hello @heisbrot interesting ! You are right the current case is a bit confusing

I am also thinking that All, Online, Offline filters complements the group filtering (you can mix both)

To complete your suggestion the dropdown with a selector before the separator and a checkbox on unassigned peers:

 ( ) All Peers
 ( ) Online Peers
 ( ) Offline Peers
 -----
 [ ] unassigned peers

I find more coherent your first suggestion, meaning keeping All, Online, Offline buttons and complete the dropdown group like this:

  [ ] groupA 
  [ ] groupB
  [ ] groupC
  -----
  [ ] unassigned peers

Right now I have this incoherent case: (I must implement an exclusivity)

Capture d’écran 2024-12-21 à 00 57 41

@EdouardVanbelle
Copy link
Author

Hello @heisbrot , I applied your suggestions, which brings:

Capture d’écran 2024-12-22 à 23 58 31 Capture d’écran 2024-12-22 à 23 58 46

Changes:

  • there is an exclusivity between Unassigned peers and other group selection to keep consistency on this selector
  • breaking change: I have also removed the All group in the drop down as all peers are always in this group, this part is arguable, up to you :)
  • the Group filter is hidden if the only one group All exists, arguable too :)
  • I have added a function overrideTableFilter() in order to keep other columns selection and reduce code redundancy. I guess there is a smarter solution but I don't know this framework
  • breaking change: take care that now the Pending Approvals button keeps other column filters (use of overrideTableFilter() function)
  • breaking change: I have also swapped buttons' order to keep filters together, and the RowsPerPage later, still up to you :)
  • the serial is still present in this merge request

🙏 all my appologies, this is the first time I code in React

@EdouardVanbelle EdouardVanbelle force-pushed the add-serial-lookup branch 2 times, most recently from 2bc4403 to 960200f Compare December 23, 2024 00:33
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