-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 standardized common SortIcon component #13987
Conversation
cursor: pointer; | ||
position: relative; | ||
color: ${theme.colors.gray[70]}; | ||
&.active { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of adding a class we can just provide the state as a prop ($active
) for the component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use this class in the tests. with a prop I wouldn't be able to pick active item
if (activeDirection === descId) return 'arrow-down-wide-short'; | ||
|
||
return 'arrow-down-wide-short'; | ||
}, [activeDirection, ascId, descId]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It probably costs more to use useMemo
here, compared with not memorising the result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe... I will change it. But descId
in condition. We might need it for the future
* Create sort Icon * Allow defining custom direction ids for common `SortIcon`. * Create sort Icon * Add sort Icon to message table and data table * Fix data table header test * Add license * Add tests * Add changelog * Refactoring * fix tsc errors Co-authored-by: Linus Pahl <linus@graylog.com>
Description
This PR adds a Sort Icon component. This component can be used in different application parts to ensure that we have the same standard.
Motivation and Context
fix: #13982
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: