You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "richardfogaca (via GitHub)" <gi...@apache.org> on 2025/11/07 00:31:44 UTC
[PR] fix(dashboard): align filter bar elements vertically in horizontal mode [superset]
richardfogaca opened a new pull request, #36036:
URL: https://github.com/apache/superset/pull/36036
### SUMMARY
Fixes vertical alignment issues in the horizontal filter bar where multiple UI elements were misaligned due to inconsistent padding and layout properties.
**Changes:**
1. **Loading spinners alignment** (`superset-ui-core/components/Loading/index.tsx`): Changed inline-centered loader from `display: block` to `display: flex` with `align-items: center` and `justify-content: center` to properly center loading spinners vertically within their containers.
2. **Action buttons alignment** (`FilterBar/ActionButtons/index.tsx`): Removed `buttonSize="small"` from the "Clear All" button to match the height of the "Apply" button, ensuring both action buttons are vertically aligned.
3. **Filter labels alignment** (`FilterBar/FilterControls/FilterControl.tsx`):
- Added `margin: 0` to `HorizontalFilterControlTitle` to remove default heading margins that were causing labels to appear lower than date
pickers
- Increased CSS specificity for `.ant-form-item-label` selector from `&& .ant-form-item-label` to `&& > .ant-row > .ant-form-item-label` to
ensure `padding-bottom: 0` overrides Ant Design's default 4px padding
4. **Filter value containers** (`FilterBar/FilterControls/FilterValue.tsx`): Made `padding-bottom` conditional based on orientation - removed padding for horizontal orientation (`FilterBarOrientation.Horizontal`) while preserving it for vertical orientation to maintain proper spacing in vertical filter bars.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
**Before:**
- Loading spinners appeared higher than filter inputs
- "Clear All" button was smaller and misaligned with "Apply" button
- Filter labels (a, v, c, d, time range) appeared lower than expected
- "More filters" button was positioned lower than filter dropdowns due to extra padding-bottom on filter labels
<img width="639" height="48" alt="Screenshot 2025-11-06 at 21 30 21" src="https://github.com/user-attachments/assets/377a5986-f033-4263-ab15-4d34d93b2e8a" />
<img width="794" height="58" alt="Screenshot 2025-11-06 at 21 30 42" src="https://github.com/user-attachments/assets/e00bbae9-a1c3-4705-8e2e-f59904912ed4" />
**After:**
- All filter controls, loading spinners, and the "More filters" button are vertically aligned at the same baseline
- Action buttons ("Apply" and "Clear All") have consistent height and alignment
- Filter labels align properly with date picker labels
<img width="204" height="57" alt="Screenshot 2025-11-06 at 20 29 48" src="https://github.com/user-attachments/assets/f7de7ddb-096a-4e4c-be28-cbb88b1cf838" />
<img width="279" height="63" alt="Screenshot 2025-11-06 at 20 30 47" src="https://github.com/user-attachments/assets/e661f542-d396-470d-bb73-5bb8567d71e0" />
<img width="395" height="95" alt="Screenshot 2025-11-06 at 21 12 42" src="https://github.com/user-attachments/assets/54b14c9c-98ab-4b1a-9965-1f06240d53fd" />
<img width="150" height="51" alt="Screenshot 2025-11-06 at 21 17 06" src="https://github.com/user-attachments/assets/9059f6b3-6697-4cce-b8ca-078bf434d07f" />
### TESTING INSTRUCTIONS
1. Navigate to a dashboard with horizontal native filters enabled
2. Verify all filter dropdowns, the "More filters" button, and action buttons are vertically aligned
3. Apply filters to trigger loading state and verify loading spinners align with the input fields
4. Check that filter labels align with date range picker labels
5. Verify "Apply" and "Clear All" buttons have the same height and vertical alignment
6. Test in vertical filter bar mode to ensure vertical spacing is preserved
### ADDITIONAL INFORMATION
- [x] Changes UI
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Includes DB Migration
- [ ] 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(dashboard): align filter bar elements vertically in horizontal mode [superset]
Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on PR #36036:
URL: https://github.com/apache/superset/pull/36036#issuecomment-3505452804
Hey @msyavuz thanks for the review. I looked at the failing test and an extra spacing on the top of the overflow filters (More filters) was removed, this caused the Filter number 12 to show entirely where it was previously partially shown. No side effects other than that.
Before the fix:
<img width="354" height="631" alt="Screenshot 2025-11-07 at 21 04 43" src="https://github.com/user-attachments/assets/fc668307-809a-4097-80a5-b71fe8b00172" />
After the fix:
<img width="290" height="585" alt="Screenshot 2025-11-07 at 21 08 02" src="https://github.com/user-attachments/assets/a207895c-ce44-4f8e-87ef-98cb41781908" />
--
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(dashboard): align filter bar elements vertically in horizontal mode [superset]
Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on PR #36036:
URL: https://github.com/apache/superset/pull/36036#issuecomment-3514325351
suggestions applied thanks @msyavuz @amaannawab923
--
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(dashboard): align filter bar elements vertically in horizontal mode [superset]
Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on PR #36036:
URL: https://github.com/apache/superset/pull/36036#issuecomment-3512434559
Can we rebase this just in case? There were some disruptive pr's merged recently
--
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(dashboard): align filter bar elements vertically in horizontal mode [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #36036:
URL: https://github.com/apache/superset/pull/36036#issuecomment-3499962371
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(dashboard): align filter bar elements vertically in horizontal mode [superset]
Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on PR #36036:
URL: https://github.com/apache/superset/pull/36036#issuecomment-3505468496
hey @msyavuz thanks for reviewing the PR. I looked into the failing test and the margin 0 kept the overflow filters closer to each other vertically, that made the filter 12 to show up entirely (instead of showing it partially). So I restored the spacing and the spacing looks the same now, thanks.
--
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(dashboard): align filter bar elements vertically in horizontal mode [superset]
Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz merged PR #36036:
URL: https://github.com/apache/superset/pull/36036
--
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(dashboard): align filter bar elements vertically in horizontal mode [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on code in PR #36036:
URL: https://github.com/apache/superset/pull/36036#discussion_r2501303317
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx:
##########
@@ -62,7 +62,10 @@ import { useFilterOutlined } from '../useFilterOutlined';
const HEIGHT = 32;
// Overrides superset-ui height with min-height
-const StyledDiv = styled.div`
+const StyledDiv = styled.div<{ orientation: FilterBarOrientation }>`
+ padding-bottom: ${({ theme, orientation }) =>
+ orientation === FilterBarOrientation.Horizontal ? 0 : theme.sizeUnit}px;
Review Comment:
### Missing null check for theme.sizeUnit <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The styled component accesses `theme.sizeUnit` without null checking, which could cause a runtime error if the theme object is undefined or doesn't contain the sizeUnit property.
###### Why this matters
If the theme context is not properly initialized or the sizeUnit property is missing, this will result in a JavaScript error when trying to access `undefined.sizeUnit`, potentially crashing the component rendering.
###### Suggested change ∙ *Feature Preview*
Add a null check and fallback value for the theme.sizeUnit property:
```typescript
const StyledDiv = styled.div<{ orientation: FilterBarOrientation }>`
padding-bottom: ${({ theme, orientation }) =>
orientation === FilterBarOrientation.Horizontal ? 0 : (theme?.sizeUnit || 4)}px;
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2538255a-78f0-478a-bedf-f21532262e87/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2538255a-78f0-478a-bedf-f21532262e87?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2538255a-78f0-478a-bedf-f21532262e87?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2538255a-78f0-478a-bedf-f21532262e87?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2538255a-78f0-478a-bedf-f21532262e87)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:1579a7f8-01b0-4b70-8919-19c032b47193 -->
[](1579a7f8-01b0-4b70-8919-19c032b47193)
--
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(dashboard): align filter bar elements vertically in horizontal mode [superset]
Posted by "amaannawab923 (via GitHub)" <gi...@apache.org>.
amaannawab923 commented on code in PR #36036:
URL: https://github.com/apache/superset/pull/36036#discussion_r2511085007
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx:
##########
@@ -52,13 +52,16 @@ const VerticalFilterControlTitle = styled.h4`
const HorizontalFilterControlTitle = styled(VerticalFilterControlTitle)`
font-weight: ${({ theme }) => theme.fontWeightNormal};
color: ${({ theme }) => theme.colorText};
+ margin: 0;
${truncationCSS};
`;
const HorizontalOverflowFilterControlTitle = styled(
HorizontalFilterControlTitle,
)`
max-width: none;
+ margin: ${({ theme }) => theme.sizeUnit * 2}px 0
Review Comment:
Descontructs of theme multiple times , can we have a single instance of theme as in other places
--
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