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

Fix for view switcher default icon display #7029

Merged

Conversation

rathodvinod5
Copy link
Contributor

Fixes #6947 (View switcher default icon display)

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This pull request addresses issue #6947 by modifying the IconPicker component and ViewPickerCreateOrEditContent to correctly display default icons for Table and Kanban views until a custom icon is selected.

  • Added takeControlOfCustomIcon prop to IconPicker for distinguishing between default and custom icons
  • Implemented getIconForIconPicker function in IconPicker to determine the correct icon to display
  • Introduced takeControlOfCustomIcon state in ViewPickerCreateOrEditContent to track custom icon selection
  • Updated IconPicker usage in ViewPickerCreateOrEditContent to respect the default icon until explicitly changed
  • Reset icon control when view type changes in ViewPickerCreateOrEditContent

2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link
Member

@charlesBochet charlesBochet 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 PR!

See my comments :)

  1. Let's not introduce a new props and behavior on IconPicker
  2. Let's set the default icon in ViewPickerCreateOrEditContentEffect instead for viewCreation case
  3. you are pushing a new yarn.lock which is incorrect could you remove this file from your commit?

@lucasbordeau
Copy link
Contributor

@rathodvinod5 The solution was to use a state in ViewPickerContentCreateMode to know if an icon we manually selected and otherwise to pass the default icon for the type to the IconPicker. But since IconPicker is a purely generic component, it didn't need to be edited.

@lucasbordeau lucasbordeau merged commit 97ab048 into twentyhq:main Oct 10, 2024
11 checks passed
harshit078 pushed a commit to harshit078/twenty that referenced this pull request Oct 14, 2024
Fixes twentyhq#6947 (View switcher default icon display)

---------

Co-authored-by: Lucas Bordeau <[email protected]>
@rathodvinod5
Copy link
Contributor Author

@lucasbordeau thanks for the guide, I will follow this practice in my future PR's.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

View Switcher - Default Icon Display
3 participants