[518] Added Table widget - #98
Conversation
|
Warning Rate limit exceeded@killev has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 10 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 Walkthrough""" WalkthroughThe changes introduce a new "Table Widget" feature to the ApostropheCMS application. This includes registering the widget module, updating main widget configurations, adding its SCSS styles, and providing both the server-side module definition and its corresponding HTML template for rendering. No existing logic or exported entities were modified. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ApostropheCMS
participant TableWidgetModule
participant UI
User->>ApostropheCMS: Requests page with table widget
ApostropheCMS->>TableWidgetModule: Fetch widget data (intro, rows, links)
TableWidgetModule->>ApostropheCMS: Returns widget data
ApostropheCMS->>UI: Renders widget.html with widget data
UI-->>User: Displays styled table widget with intro, rows, and links
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (14)
website/modules/asset/ui/src/scss/_table-widget.scss (6)
1-12: Consider centering the intro block.You’ve set a
max-width: 800pxon.sf-table-intro, but without horizontal centering it will always hug the left edge.
Suggestion: addmargin: 0 auto;to center within its container.
14-23: Leverage CSS gap for flexbox items.Instead of manual margins or wrapping logic at the medium breakpoint, you can simplify spacing between rows by using
gap. For example:.sf-table { display: flex; flex-wrap: wrap; gap: 24px; @include breakpoint-medium { flex-direction: row; } }This eliminates the need for negative margins and manual spacing adjustments.
25-55: Simplify redundant nth-child selectors.You’ve applied three selectors to remove the top border on break-row items:
&:nth-child(3n+6), &:nth-child(4n), &:nth-child(n+4) { border-top: none; }The single rule
&:nth-child(n+4)already matches every item from the 4th onward. Consider dropping the other two for clarity and maintainability.
57-65: Unify title-cell typography styling.Both
.sf-table-cell-titleand.sf-table-titleapplyfont-weight: 700and adjust font-size at the same breakpoint. You can merge these into a single shared mixin or move the typography rules to one class to eliminate duplication.
67-76: Avoid duplicate breakpoint font-size rules.
.sf-table-titlebumps from 18px→22px at medium, while.sf-table-cell-titlegoes 14px→22px. Given they converge at 22px, consider deriving both from a shared base, or adjust mobile sizes for a clearer hierarchy.
96-112: Consolidate link styling and class usage.You’re styling links via
.sf-table-link abut your template also adds asf-linkclass to each<a>. Consider removing the extra class and relying solely on the parent selector for consistency:- <a href="{{ linkUrl }}" class="sf-link" …> + <a href="{{ linkUrl }}" …>Or, if
sf-linkis needed elsewhere, update your SCSS to target.sf-linkdirectly.website/modules/asset/ui/src/index.scss (1)
23-23: Maintain import naming consistency.Other partials include the
.scssextension (_cases.scss,_container.scss). For consistency, consider:- @import './scss/_table-widget'; + @import './scss/_table-widget.scss';This makes it clear at a glance that you’re importing an SCSS file.
website/lib/mainWidgets.js (1)
22-22: Optional: Alphabetize widget list.You’ve added
'table': {}inline—consider sorting thewidgetskeys alphabetically to make future maintenance and diff reviews easier.website/modules/table-widget/views/widget.html (2)
1-6: Enhance accessibility with ARIA roles.Because you’re using
divfor tabular data, consider adding ARIA roles to improve screen-reader support:<section class="sf-section" data-table-widget role="table"> {% if data.widget.intro %} <div class="sf-table-intro" role="rowgroup"> … </div> {% endif %} … </section>Or evaluate using a semantic
<table>if that better fits your content.
8-37: Consider semantic markup for tabular data.This structure is visually a table, but you’re using flexbox. For improved semantics and keyboard navigation, you could switch to:
<table class="sf-table"> <thead>…intro…</thead> <tbody> {% for row in data.widget.rows %} <tr> <th scope="row">{{ row.title }}</th> <td>{% area row, 'description' %}{% if linkUrl %}<a href="{{ linkUrl }}">…</a>{% endif %}</td> </tr> {% endfor %} </tbody> </table>This brings native accessibility benefits without extra ARIA roles.
website/modules/table-widget/index.js (4)
49-102: Consider adding conditional validation for link fieldsWhile the
linkTypefield is optional, if a user selects a link type, there's no validation to ensure they complete the corresponding link field (page, file, or custom URL).Consider adding validation or making the corresponding fields required when a link type is selected:
_page: { label: 'Link to page', type: 'relationship', withType: '@apostrophecms/page', max: 1, builders: { project: { title: 1, _url: 1, }, }, if: { linkType: 'page', }, + required: true, },Similar changes could be applied to
_fileandcustomUrlfields.
87-95: Consider adding projection to file relationshipThe
_pagerelationship includes a projection to limit the fields fetched, but the_filerelationship doesn't have a similar optimization.For consistency and performance, consider adding a projection to the file relationship:
_file: { label: 'Link to file', type: 'relationship', withType: '@apostrophecms/file', max: 1, + builders: { + project: { + title: 1, + attachment: 1, + }, + }, if: { linkType: 'file', }, },
103-112: Consider showing target option only when a link is definedCurrently, the "Open in a new tab" option appears regardless of whether a link is defined, which might confuse users.
Consider showing this field only when a link type is selected:
target: { label: 'Will the link open a new browser tab?', type: 'checkboxes', + if: { + $or: [ + { linkType: 'page' }, + { linkType: 'file' }, + { linkType: 'custom' } + ] + }, choices: [ { label: 'Open in a new tab', value: '_blank', }, ], },
1-118: Consider adding JSDoc comments for better documentationWhile the code is clear, adding JSDoc comments would improve maintainability and help other developers understand the widget's purpose and structure.
For example, at the top of the file:
/** * Table Widget module for ApostropheCMS * * This widget displays information in a table format with optional intro text. * Each row can have a title, rich text description, and optional link. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
website/app.js(1 hunks)website/lib/mainWidgets.js(1 hunks)website/modules/asset/ui/src/index.scss(1 hunks)website/modules/asset/ui/src/scss/_table-widget.scss(1 hunks)website/modules/table-widget/index.js(1 hunks)website/modules/table-widget/views/widget.html(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
website/modules/table-widget/index.js (1)
website/lib/mainWidgets.js (1)
headingToolbar(1-1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: e2e-tests
- GitHub Check: security-scan
🔇 Additional comments (6)
website/modules/asset/ui/src/scss/_table-widget.scss (1)
78-93: Mobile-first description styling looks good.The font-weight, sizing, and adaptive
min-heighton medium breakpoints are well thought out. This block is straightforward and aligns with the design.website/app.js (1)
122-122: Widget module registered correctly.The addition of
'table-widget': {}inmodulesensures the new Table Widget is loaded by Apostrophe. No action needed here.website/modules/table-widget/index.js (4)
1-3: Imports look goodThe imports for
extendedToolbarandheadingToolbarare correctly structured and consistent with other parts of the codebase.
4-9: Widget configuration is clear and follows best practicesThe module correctly extends the base widget type with appropriate label and icon properties.
10-24: Intro field configuration looks goodThe optional intro field is well configured with appropriate constraints and toolbar options.
25-48: Table rows structure is well-definedThe table rows configuration with required title and description fields provides a solid foundation for the widget.
🔍 Vulnerabilities of
|
| digest | sha256:90dda227c64b59dce2a79a7d79ab7f8d5fc2d87e51d84e01a6f57a808abf2b75 |
| vulnerabilities |     |
| platform | linux/amd64 |
| size | 284 MB |
| packages | 935 |
📦 Base Image node:23-alpine
| also known as |
|
| digest | sha256:169ee1b69bd3f5c7c8508a9919e8b367bba9dc7fa9a03097ec9f85d2e8179631 |
| vulnerabilities |  |
Description
| ||||||||||||
Description
|
|
5 participants
Added new Table widget