-
Notifications
You must be signed in to change notification settings - Fork 1
feat(ngx-table): add an optional context row to the table #446
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
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.
LGTM!
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.
Ik mis wel wat context, maar code-wise ziet het er wel goed uit. 😅
ccff7c6
to
9f2d979
Compare
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.
In general, I think both this context & detail feature have too much overlap. Consider splitting the way you render your row to a separate component and allowing implementations to overwrite / extend the specifics.
This obviously exceeds the scope of this change and warrants a breaking change but does provide more flexiblit. From what I understand, at the time of writing, the only difference between context & detail is top & bottom respectively.
? 'ngx-table-row-first' | ||
: !contextRowClasses[dataIndex] && dataIndex === 0 | ||
? 'ngx-table-row-first' | ||
: '', |
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.
These inline if-statements (also in other parts of the code) are getting out of hand. Consider opening an extra issue to split them to utils / consts / ... to preserve legibility
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.
You are absolutely correct. If and when I find the time, I will analyse this a bit further
Description
The table has gotten an upgrade!
We required a context row for the VLAIO project and saw no other practical way than to add it as a feature to the table.
In short: a context row is a row that adds no accessibility addition, will always be interpreted as if it was a
span
element. The text in it will be read, buttons and links will have their persisting accessibility features, and an additional row is added to the table. This row allows for a more customisable experience, like the one in the styled example of the docs. (added as PDF to the attachments)It uses an input and is therefore toggleable. It uses keys defined in the data object array to evaluate whether a row requires an additional context header.
Requirements
PDF of the docs:
context-row-docs.pdf
Attachments
Screen.Recording.2025-07-30.at.18.14.44.mov