You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "gabotorresruiz (via GitHub)" <gi...@apache.org> on 2025/10/02 21:13:29 UTC
[PR] fix(table-chart): spacings between header tools [superset]
gabotorresruiz opened a new pull request, #35460:
URL: https://github.com/apache/superset/pull/35460
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
This PR improves the `Table Chart` header controls in several ways:
1. **Fixed accessibility issue**: Removed non-functional `sr-only` class that was causing "Select page size" label to be visible. The class doesn't exist in the codebase, so the label was showing alongside the intended UI.
2. **Improved responsive layout**: Replaced multiple `Space` components with Ant Design's `Flex` component for better control wrapping behavior. The controls now properly adapt to different chart widths when resized in dashboards.
3. **Layout structure**:
- Left side: "Show X entries per page" selector
- Right side: `Search by` dropdown and `Search` input
- Controls wrap gracefully when space is limited
- All elements use proper theming tokens (`theme.sizeUnit`)
4. **Code cleanup**: Removed unnecessary styled components and simplified the component structure using built-in `Ant Design` components.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
Before:
<img width="589" height="412" alt="image (5)" src="https://github.com/user-attachments/assets/90d0ab1e-377c-49ea-bc09-c3c3414e2a0b" />
After:
<img width="1461" height="415" alt="Screenshot 2025-10-02 at 16 57 58" src="https://github.com/user-attachments/assets/2dbcba51-8bb9-4f7c-ab65-bfd6a052b829" />
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
1. Create (or open) a Table Chart with `server-side pagination` and `Search Box` enabled
2. Verify the header shows "Show X entries per page" on the left
3. Verify `Search by` dropdown and `Search` input appear on the right
4. Add the chart to a dashboard
5. Resize the chart to different widths and verify:
- Large width: controls stay on one row (page size left, search controls right)
- Medium width: controls wrap to separate rows
- Small width: all controls stack vertically
6. Verify no "Select page size" text appears anywhere
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(table-chart): spacings between header tools [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35460:
URL: https://github.com/apache/superset/pull/35460#issuecomment-3364385788
🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/18214191591) for [1567b2d](https://github.com/apache/superset/commit/1567b2d)
• **Environment:** http://35.92.204.22:8080 (admin/admin)
• **Lifetime:** 48h auto-cleanup
• **Updates:** New commits create fresh environments automatically
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(table-chart): spacings between header tools [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #35460:
URL: https://github.com/apache/superset/pull/35460#issuecomment-3363082030
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(table-chart): spacings between header tools [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35460:
URL: https://github.com/apache/superset/pull/35460#issuecomment-3364367729
🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/18214191591) for [1567b2d](https://github.com/apache/superset/commit/1567b2d)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(table-chart): code improvements + component refactor [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #35460:
URL: https://github.com/apache/superset/pull/35460#issuecomment-4700121205
This still has some big merge conflicts and gone quiet, and at 48 files it's tough to review safely as one chunk. Closing for now - if you want to revive it, splitting the a11y fix and control changes from the pure reorg would make it far easier to land. Just ping me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(table-chart): spacings between header tools [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #35460:
URL: https://github.com/apache/superset/pull/35460#discussion_r2400090566
##########
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:
##########
@@ -225,13 +225,9 @@ function SelectPageSize({
const { Option } = Select;
return (
- <>
- <label htmlFor="pageSizeSelect" className="sr-only">
- {t('Select page size')}
- </label>
+ <span className="dt-select-page-size">
{t('Show')}{' '}
<Select<number>
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Accessibility compliance broken</b></div>
<div id="fix">
The PR removes the accessibility label and id from the SelectPageSize component, breaking WCAG 2.1 compliance and screen reader support. The `label` with `htmlFor="pageSizeSelect"` provides critical association between the label and the `Select` component for assistive technologies. This affects downstream usage in `DataTable` component where `SelectPageSize` is rendered as `selectPageSize` prop. Restore the hidden label for screen readers and maintain the id association to ensure the dropdown remains accessible.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```suggestion
<label htmlFor="pageSizeSelect" className="sr-only">
{t('Show')} {t('Select page size')}
</label>
{t('Show')} {' '}
<Select<number> id="pageSizeSelect"
```
</div>
</details>
</div>
<small><i>Code Review Run <a href=https://github.com/apache/superset/pull/35460#issuecomment-3363082030>#a735d3</a></i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(table-chart): code improvements + component refactor [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas closed pull request #35460: fix(table-chart): code improvements + component refactor
URL: https://github.com/apache/superset/pull/35460
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org