feat(custom): implement SubscriptionTable scaffolding and register global module exports - #1660
Conversation
…ule exports Signed-off-by: Abhinav Deval <abhinavkdeval08@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new SubscriptionTable component to compare free, team, and enterprise subscription plans, along with its custom styled components. The feedback suggests integrating the newly defined custom styled components from style.tsx into the main table component to clean up unused Material-UI imports, and exposing hardcoded UI strings as configurable props to support internationalization (i18n).
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
434a2c2 to
b7298c4
Compare
…ule exports Signed-off-by: Abhinav Deval <abhinavkdeval08@gmail.com>
56442e9 to
812e1ac
Compare
Bhumikagarggg
left a comment
Bhumikagarggg
left a comment
There was a problem hiding this comment.
@abhinavkdeval08-design Please review all the suggestions raised by Copilot. If a suggestion is valid, please address it. If not, please reply to the comment explaining why no change is needed so that all comments are resolved.
Signed-off-by: Abhinav Deval <abhinavkdeval08@gmail.com>
b1b798c to
43af05b
Compare
|
@Bhumikagarggg @leecalcote @KhushamBansal |
@abhinavkdeval08-design consider and reply to all feedback offered. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughAdds a themed ChangesSubscription table
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change adds and exports a localized subscription comparison UI component without introducing service, permission, persistence, or deployment behavior changes; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Consumer
participant SubscriptionTable
participant onPlanSelect
Consumer->>SubscriptionTable: provide features and plan labels
SubscriptionTable->>Consumer: render plan buttons and feature values
Consumer->>SubscriptionTable: select a plan
SubscriptionTable->>onPlanSelect: invoke selected plan type
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 6 files. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/custom/SubscriptionTable/style.tsx`:
- Around line 13-29: Replace raw colors and font-family/weight overrides with
Sistent theme palette and typography tokens across all affected sites: in
src/custom/SubscriptionTable/style.tsx lines 13-29, update StyledHeaderRow,
StyledTableCell, and FeatureHeaderCell to derive colors and typography from the
theme, including theme.palette.text.primary or its Sistent equivalent; in
src/custom/SubscriptionTable/SubscriptionTable.tsx lines 49-65 and 74-104, apply
the appropriate Sistent typography token to the feature and plan header cells
instead of literal family and weight values.
- Around line 5-10: Update the StyledTableContainer styles to allow horizontal
scrolling on narrow viewports instead of clipping the table with overflow
hidden. Preserve the existing visual styles and ensure the 650px-minimum-width
table remains accessible, including later plan columns.
In `@src/custom/SubscriptionTable/SubscriptionTable.tsx`:
- Around line 40-52: Add an accessible text alternative to both boolean branches
in renderValue: label the CheckIcon for the included state and the CloseIcon for
the excluded state using titleAccess, aria-label, or visually hidden text, while
preserving their existing visual styling and test IDs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a195d272-9565-414b-9f9a-519c7ff87d0c
📒 Files selected for processing (5)
src/custom/SubscriptionTable/SubscriptionTable.tsxsrc/custom/SubscriptionTable/index.tssrc/custom/SubscriptionTable/style.tsxsrc/custom/index.tssrc/custom/index.tsx
KhushamBansal
left a comment
KhushamBansal
left a comment
There was a problem hiding this comment.
@abhinavkdeval08-design Please resolve merge conflicts.
Signed-off-by: Abhinav Deval <abhinavkdeval08@gmail.com>
…cessibility Signed-off-by: Abhinav Deval <abhinavkdeval08@gmail.com>
|
@KhushamBansal @Bhumikagarggg Resolved the merge conflicts with master and addressed all review feedback regarding responsive horizontal overflow, Sistent theme/typography tokens, and accessibility labels (titleAccess). CodeRabbit checks are green. Ready for review and merge! |
PARTH-TUSSLE
left a comment
PARTH-TUSSLE
left a comment
There was a problem hiding this comment.
Hey @abhinavkdeval08-design , pls reply to all the bot suggestions stating how you addressed that feedback or a valid reason if rejecting it and then click on "resolve conversation".
|
Hi @PARTH-TUSSLE @KhushamBansal @Bhumikagarggg, All bot suggestions have been addressed (commit
All DCO and CI checks are passing green. Ready for your final approval and merge! |
There was a problem hiding this comment.
Hi @PARTH-TUSSLE @KhushamBansal @Bhumikagarggg,
All bot suggestions have been addressed (commit
191ae3b) and all review conversations are now resolved.
- Unused imports cleaned up and custom styled wrappers integrated.
- Sistent theme typography and semantic palette tokens applied across table cells.
- i18n support enabled with configurable label props and default fallbacks.
- Horizontal overflow added for responsive mobile viewports.
- Accessibility labels (
titleAccess) added to boolean indicators.All DCO and CI checks are passing green. Ready for your final approval and merge!
Nope, you still haven't replied to the bot conversations before resolving them ^ ^
|
Hi @PARTH-TUSSLE, All inline review drafts have been submitted and all bot conversation threads are marked resolved under Files Changed. All checks and DCO are green. Ready for your review and approval! |
Missed this one :) |
|
@PARTH-TUSSLE Done bro :) |
There was a problem hiding this comment.
@PARTH-TUSSLE Done bro :)
Still can't see the reply on it , maybe I'm blind :), anyways I don't wanna be annoying. I reviewed the PR and noticed, since SubscriptionTable is a new public component, can you add an explicit root export in src/index.tsx for SubscriptionTable and its public types (SubscriptionTableProps / PlanFeature)? The repo has a known issue where exports that only come through export * from './custom' can be missing from the generated dist/index.d.ts.
also a small SubscriptionTable.test.tsx covering the basic rendering, boolean/string values, plan selection callback, and custom labels would be good. Nothing too extensive, just enough regression coverage for the new public component.
Other than that, the implementation looks good to me. Once those are addressed(and you've replied to all the bot conversations T-T , I promise this is the last time I'm pointing this out :) ), I'm happy to approve.
…nTable Signed-off-by: Abhinav Deval <abhinavkdeval08@gmail.com>
|
Hi @PARTH-TUSSLE, I have pushed the requested updates in commit
All CI checks and DCO are green. Ready for your final approval! |
5 participants
Description
This PR addresses issue #606 by implementing the missing structural baseline scaffolding for the new
SubscriptionTablecomponent pattern wrapper matrix within the Sistent design system (src/custom).Changes Proposed
src/custom/SubscriptionTable/directory module.SubscriptionTable.tsxutilizing strict type definitions (PlanFeatureprops) to dynamically map features.style.tsxutilizing specialized MUI styled engines and Figma typography token overrides (Qanelas SoftandOpen Sans).src/custom/index.tsandsrc/custom/index.tsx) to expose the API interface.Notes for Reviewers
Signed commits
cc @KhushamBansal @leecalcote @Bhumikagarggg
Summary by CodeRabbit