✨ #11 - feat: add datagrid component #18
Conversation
svenvandescheur
force-pushed
the
feature/#11-grid
branch
from
c9d9338 to
4b385e6
Compare
svenvandescheur
force-pushed
the
feature/#11-grid
branch
2 times, most recently
from
bfff84e to
e64b0b6
Compare
There was a problem hiding this comment.
I have done some code checks, visually:
- We are missing checkboxes to select each datagrid, and the button to do something with the selected ones. Are we leaving this aside for now?
- The table headers are not aligned correctly according to the design.
- Could you take another look at the mobile design, the aligning of the values seems a bit unnatural to me.
- Upon closer inspection, the borders on the pagination buttons are always lit up (blueish), but in the design they should be more greyed out. Any thoughts on this?
| &__row { | ||
| background-color: var( | ||
| --typography-color-background | ||
| ); //border-radius: var(--border-radus-m); |
src/components/datagrid/datagrid.tsx
Outdated
| */ | ||
| const renderCell = (rowData: RowData, field: string, index: number) => { | ||
| const fieldIndex = renderableFields.indexOf(field); | ||
| const urlField = urlFields.find((f) => String(rowData[f]).match(REGEX_URL)); |
There was a problem hiding this comment.
Is it possible to make a re-usable function in libs for this?
There was a problem hiding this comment.
I moved field2caption to libs, extracting the urlField seems to be very specific logic to me. I don't think we should move that to libs for now.
Update: isLink created in libs
| : isImplicitLink | ||
| ? String(rowUrl) | ||
| : ""; | ||
|
|
There was a problem hiding this comment.
I understand why we are using the ternaries here, but it is very unreadable. Could we slightly refactor this for readability?
| /** | ||
| * FIXME: This effectively still uses index as keys which might lead to issues. | ||
| * @see {@link https://react.dev/learn/rendering-lists#rules-of-keys|Rules of keys} | ||
| */ |
There was a problem hiding this comment.
Will do so in a future PR containing the sorting functionality.
src/components/datagrid/datagrid.tsx
Outdated
| * Converts "field_name" to "FIELD NAME". | ||
| * @param field | ||
| */ | ||
| const field2Caption = (field: string): string => |
There was a problem hiding this comment.
Should we make a function in lib for this?
Yes, they might be added in the future but we postponed that feature for now.
|
svenvandescheur
force-pushed
the
feature/#11-grid
branch
2 times, most recently
from
789c488 to
f0bd585
Compare
svenvandescheur
force-pushed
the
feature/#11-grid
branch
from
f0bd585 to
888dc0f
Compare
|
@Xaohs merging to unblock, additional review is still appreciated. |
Ready for review