-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat(adminPanel): RN-1301: Enable column resizing #5663
Conversation
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.
Looks good! Great change!
I reckon this warrants a feat
over a tweak
💪
One thing I noticed when playing with it is that it felt a bit easy to accidentally click the header cell and sort the column rather than re-size it. If the sort button was reduced to just the column name (or even just the sort icon) it might feel easier to use. But not sure if that's easy/doable and it's just my opinion so pre-approving 👍
border: none; | ||
height: 1px; // need this to make the cell content fill the height of the cell | ||
position: relative; | ||
// height: 1px; // need this to make the cell content fill the height of the cell |
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.
// height: 1px; // need this to make the cell content fill the height of the cell |
Do we not need this anymore?
@@ -54,7 +53,8 @@ const COLUMNS = [ | |||
{ | |||
Header: 'Test', | |||
type: 'testDatabaseConnection', | |||
colWidth: '6.5rem', |
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.
Was this not working previously? Or preferring to switch to static width because it's related to expected character lengths of this field?
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.
Since the columns are resizable, we rely now on react-table
to give use the width, and they only support px
values, not strings (e.g rems
) so have changed to a width
and a pixel value instead
Issue RN-1301: Enable column resizing
Changes:
tooltip