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

style(filter-component): redesign filter tag ui #861

Merged
merged 4 commits into from
Jun 25, 2023
Merged

style(filter-component): redesign filter tag ui #861

merged 4 commits into from
Jun 25, 2023

Conversation

shobhitexe
Copy link
Contributor

@shobhitexe shobhitexe commented Jun 18, 2023

Closes #841

Problem

The filter tags on inventory is looking more of a buttons and blending in the background

Solution

Changes the style of tags as per figma design

Changes Made

  • Updated the layout and styling of the filter tags.
  • Changed the positioning of the button from absolute to flex to align it within the component.

Screenshots

2023-06-18.14-40-51.mp4

Checklist

  • Code follows the contributing guidelines
  • Changes have been thoroughly tested
  • Documentation has been updated, if necessary
  • Any dependencies have been added to the project, if necessary

Reviewers

@mlabouardy

@ShubhamPalriwala
Copy link
Contributor

Hey @shobhitexe, thanks for the PR! 🚀 amazing work and really appreciate the screen recording! The changes look great.
However, I think, you missed the "+" symbol to add filters right next to the current filters as well?

Could you let me know what you think of that? Thanks

@shobhitexe
Copy link
Contributor Author

Hey @shobhitexe, thanks for the PR! 🚀 amazing work and really appreciate the screen recording! The changes look great. However, I think, you missed the "+" symbol to add filters right next to the current filters as well?

Could you let me know what you think of that? Thanks

Thank you for your feedback and appreciation, I apologize for overlooking the addition of the "+" symbol to add filters next to the current filters. I appreciate you bringing it to my attention.

Regarding your question, I can make the necessary changes and include the "+" button in the current pull request. This way, we can keep all the related changes together in one place.
Alternatively, if you prefer, I can create a new pull request specifically for the addition of the "+" button. Please let me know your preference, and I will proceed accordingly.

Once again, thank you for your valuable feedback, and I'll make sure to address the missing "+" button promptly.

@ShubhamPalriwala
Copy link
Contributor

Adding it here would be just perfect!🙌🏻 would be happy to review once pushed!

Copy link
Collaborator

@mlabouardy mlabouardy left a comment

Choose a reason for hiding this comment

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

Great job @shobhitexe, LGTM!
Just missing the "+" action as mentioned by @ShubhamPalriwala :)

@mlabouardy mlabouardy added the ui label Jun 19, 2023
@mlabouardy mlabouardy added this to the v3.0.20 milestone Jun 19, 2023
@shobhitexe
Copy link
Contributor Author

shobhitexe commented Jun 20, 2023

Hello I have implemented the functionality for "+" button Please review the pull request as I have made commits in the same branch.

  • included an add icon and a separator div.

  • Initially, the dropdown menu and the "Filter by" button were combined into the same component, which limited dropdown component reusability. To address this, I separated the dropdown into its own component, allowing it to be reused multiple times, such as in the add button.

2023-06-20.16-38-26.mp4

@mlabouardy mlabouardy self-requested a review June 21, 2023 10:21
Copy link
Collaborator

@mlabouardy mlabouardy left a comment

Choose a reason for hiding this comment

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

Thanks for the changes @shobhitexe, I've identified a few bugs that need to be fixed :)

  • The "+" button should be removed from a custom view as we don't allow users to update filters of a saved view:
Screen.Recording.2023-06-21.at.12.21.34.mov
  • Might be related to the first bug but if you set up a filter on a view and then you jump back to "All resources", the previous values of the filter at set in the dropdown:
Screen.Recording.2023-06-21.at.12.24.18.mov
  • I like that you keep the filters dropdown menu when a filter is added :) but can we close it if the filter was added from the "Filter by" button and not from the "+" button?
Screen.Recording.2023-06-21.at.12.28.59.mov

@shobhitexe
Copy link
Contributor Author

Thanks for the changes @shobhitexe, I've identified a few bugs that need to be fixed :)

  • The "+" button should be removed from a custom view as we don't allow users to update filters of a saved view:

Screen.Recording.2023-06-21.at.12.21.34.mov

  • Might be related to the first bug but if you set up a filter on a view and then you jump back to "All resources", the previous values of the filter at set in the dropdown:

Screen.Recording.2023-06-21.at.12.24.18.mov

  • I like that you keep the filters dropdown menu when a filter is added :) but can we close it if the filter was added from the "Filter by" button and not from the "+" button?

Screen.Recording.2023-06-21.at.12.28.59.mov

Thanks for the changes @shobhitexe, I've identified a few bugs that need to be fixed :)

  • The "+" button should be removed from a custom view as we don't allow users to update filters of a saved view:

Screen.Recording.2023-06-21.at.12.21.34.mov

  • Might be related to the first bug but if you set up a filter on a view and then you jump back to "All resources", the previous values of the filter at set in the dropdown:

Screen.Recording.2023-06-21.at.12.24.18.mov

  • I like that you keep the filters dropdown menu when a filter is added :) but can we close it if the filter was added from the "Filter by" button and not from the "+" button?

Screen.Recording.2023-06-21.at.12.28.59.mov

Sure , as per i understood

  • When in a saved view, there should not be any filters buttons present.
  • The filters from different saved views are being saved, ( but they can be removed if we remove the filter buttons from the saved view. related to bug 1 )
  • The "Filter By" button should only accept one filter at a time and then close, instead of staying open and waiting for additional filters.

Am i correct ?

@mlabouardy
Copy link
Collaborator

Thanks for the changes @shobhitexe, I've identified a few bugs that need to be fixed :)

  • The "+" button should be removed from a custom view as we don't allow users to update filters of a saved view:

Screen.Recording.2023-06-21.at.12.21.34.mov

  • Might be related to the first bug but if you set up a filter on a view and then you jump back to "All resources", the previous values of the filter at set in the dropdown:

Screen.Recording.2023-06-21.at.12.24.18.mov

  • I like that you keep the filters dropdown menu when a filter is added :) but can we close it if the filter was added from the "Filter by" button and not from the "+" button?

Screen.Recording.2023-06-21.at.12.28.59.mov

Thanks for the changes @shobhitexe, I've identified a few bugs that need to be fixed :)

  • The "+" button should be removed from a custom view as we don't allow users to update filters of a saved view:

Screen.Recording.2023-06-21.at.12.21.34.mov

  • Might be related to the first bug but if you set up a filter on a view and then you jump back to "All resources", the previous values of the filter at set in the dropdown:

Screen.Recording.2023-06-21.at.12.24.18.mov

  • I like that you keep the filters dropdown menu when a filter is added :) but can we close it if the filter was added from the "Filter by" button and not from the "+" button?

Screen.Recording.2023-06-21.at.12.28.59.mov

Sure , as per i understood

  • When in a saved view, there should not be any filters buttons present.
  • The filters from different saved views are being saved, ( but they can be removed if we remove the filter buttons from the saved view. related to bug 1 )
  • The "Filter By" button should only accept one filter at a time and then close, instead of staying open and waiting for additional filters.

Am i correct ?

exactly :)

@shobhitexe
Copy link
Contributor Author

Hello, I have made some changes to address the bugs. Could you please take a look

  • The visibility of the filter in saved views has been fixed so that it is no longer visible in saved views.
2023-06-22.18-13-10.mp4
  • The previous values of the filter summary are no longer stored from previous filters. ( implemented useEffect hook to clear the previous values on every render of the dropdown. )
2023-06-22.18-13-38.mp4
  • The filter button now accepts only one submit and closes, while the add button can accept multiple submits and remains open.
2023-06-22.18-15-23.mp4

Copy link
Contributor

@ShubhamPalriwala ShubhamPalriwala left a comment

Choose a reason for hiding this comment

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

Amazing work here @shobhitexe! Love the minute details that you have handled! Lgtm. Over to you @mlabouardy 🚀

@mlabouardy mlabouardy self-requested a review June 25, 2023 18:57
Copy link
Collaborator

@mlabouardy mlabouardy left a comment

Choose a reason for hiding this comment

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

Thanks @shobhitexe for the changes 🙏🏻

@mlabouardy mlabouardy merged commit 7141d04 into tailwarden:develop Jun 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filters bar & filters style don't match the designs
3 participants