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

Refactor/node list groups #2166

Merged
merged 12 commits into from
Nov 4, 2024
Merged

Refactor/node list groups #2166

merged 12 commits into from
Nov 4, 2024

Conversation

Huongg
Copy link
Contributor

@Huongg Huongg commented Oct 31, 2024

Description

Fixes #2153

Main changes are as below:

  • Separate the node-list-groups, node-list-group, and node-list-row-list from node-list
  • Rename them to be "Filters" with the new structure as in the chart below
Screenshot 2024-10-31 at 13 53 22

Development notes

QA notes

Checklist

  • Read the contributing guidelines
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added new entries to the RELEASE.md file
  • Added tests to cover my changes

Huong Nguyen added 9 commits October 30, 2024 11:54
Create new structure and its own folder for filters or groups
48ed5d8 
Signed-off-by: Huong Nguyen <huongg1409@gmail>
better names for component structure
d122344 
Signed-off-by: Huong Nguyen <huongg1409@gmail>
FiltersSectionHeading
94128a2 
Signed-off-by: Huong Nguyen <huongg1409@gmail>
filters-section
06d19a3 
Signed-off-by: Huong Nguyen <huongg1409@gmail>
filters component
83f2c12 
Signed-off-by: Huong Nguyen <huongg1409@gmail>
filtersSectionHeading component
8fb6fa3 
Signed-off-by: Huong Nguyen <huongg1409@gmail>
tidy up code
9b2afbe 
Signed-off-by: Huong Nguyen <huongg1409@gmail>
including new tests for new components
bceae87 
Signed-off-by: Huong Nguyen <huongg1409@gmail>
update and remove existing tests
1df0dd3 
Signed-off-by: Huong Nguyen <huongg1409@gmail>
@Huongg Huongg marked this pull request as ready for review October 31, 2024 13:55
rashidakanchwala
rashidakanchwala reviewed Oct 31, 2024
View reviewed changes
Copy link
Contributor

@rashidakanchwala rashidakanchwala left a comment

Choose a reason for hiding this comment

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

Can we have a filters folder, and everything within that filter-group, filter-row, and filter section?

@Huongg
Copy link
Contributor Author

Huongg commented Oct 31, 2024 •
edited
Loading

Can we have a filters folder, and everything within that filter-group, filter-row, and filter section?

hey @rashidakanchwala everything is currently in the Filters folder at the moment? Unless you want to move it to somewhere else?

Screenshot 2024-10-31 at 14 13 26

@rashidakanchwala
Copy link
Contributor

nice, i got confused coz of the components folder!

remove un-used variables
a22649a 
Signed-off-by: Huong Nguyen <huongg1409@gmail>
@Huongg Huongg self-assigned this Oct 31, 2024
rashidakanchwala
rashidakanchwala approved these changes Nov 1, 2024
View reviewed changes
Copy link
Contributor

@rashidakanchwala rashidakanchwala left a comment

Choose a reason for hiding this comment

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

LGTM. very nice. thanks!

SajidAlamQB
SajidAlamQB approved these changes Nov 1, 2024
View reviewed changes
Copy link
Contributor

@SajidAlamQB SajidAlamQB left a comment

Choose a reason for hiding this comment

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

Awesome work @Huongg the refactor looks great!

nit: it might be cleaner to remove the extra components folder within filters to keep the directory structure flatter, wdyt?

@rashidakanchwala
Copy link
Contributor

nit: it might be cleaner to remove the extra components folder within filters to keep the directory structure flatter, wdyt?

I second that !

@Huongg
Copy link
Contributor Author

Huongg commented Nov 1, 2024

Awesome work @Huongg the refactor looks great!

nit: it might be cleaner to remove the extra components folder within filters to keep the directory structure flatter, wdyt?

I don't mind I can remove the components folder 👍

@jitu5
Copy link
Contributor

jitu5 commented Nov 1, 2024 •
edited
Loading

@Huongg
[nit] Two minor observations.

  1. For tag, group toggle icon is always visible compared to demo site
  2. Selected blue color is little different compared to demo site
Screenshot 2024-11-01 at 3 59 40 p m

Demo site
Screenshot 2024-11-01 at 4 00 38 p m

jitu5
jitu5 approved these changes Nov 1, 2024
View reviewed changes
Copy link
Contributor

@jitu5 jitu5 left a comment

Choose a reason for hiding this comment

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

This is really clean, thanks @Huongg.

@Huongg
Copy link
Contributor Author

Huongg commented Nov 4, 2024

@Huongg [nit] Two minor observations.

  1. For tag, group toggle icon is always visible compared to demo site
  2. Selected blue color is little different compared to demo site

hey @jitu5 what you see in the demo site is a bug, the box should always visible, so I fixed it in this PR now

Huong Nguyen added 2 commits November 4, 2024 09:26
remove components folder
0f4ad22 
Signed-off-by: Huong Nguyen <huongg1409@gmail>
update tests path
81a100d 
Signed-off-by: Huong Nguyen <huongg1409@gmail>
@Huongg Huongg merged commit 61c1de3 into feature-branch/refactor-node-list Nov 4, 2024
4 checks passed
@Huongg Huongg deleted the refactor/node-list-groups branch November 4, 2024 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SajidAlamQB SajidAlamQB SajidAlamQB approved these changes

@jitu5 jitu5 jitu5 approved these changes

@rashidakanchwala rashidakanchwala rashidakanchwala approved these changes

@ravi-kumar-pilla ravi-kumar-pilla Awaiting requested review from ravi-kumar-pilla

Assignees

@Huongg Huongg

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Huongg @rashidakanchwala @jitu5 @SajidAlamQB