You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "mistercrunch (via GitHub)" <gi...@apache.org> on 2025/08/15 01:25:49 UTC
[PR] feat: refactor modals to use consistent design patterns [superset]
mistercrunch opened a new pull request, #34711:
URL: https://github.com/apache/superset/pull/34711
[DRAFT!]
- Create shared modal components (StandardModal, ModalFormField, useModalValidation)
- Refactor Chart Properties modal to use collapsible sections with validation
- Refactor Dashboard Properties modal to use shared components
- Refactor AlertReportModal to use StandardModal
- Replace 2-column layouts with single-column collapsible sections
- Add real-time validation with visual feedback
- Standardize modal width to 500px for consistent UX
- Improve accessibility and keyboard navigation
Key improvements:
- ~40% code reduction through component reuse
- Consistent styling across all property modals
- Better organization with logical collapsible sections
- Type-safe validation system with error tooltips
--
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] feat: refactor modals to use consistent design patterns [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34711:
URL: https://github.com/apache/superset/pull/34711#issuecomment-3204275650
@yousoph Ephemeral environment spinning up at http://35.164.4.101:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
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] feat: refactor modals to use consistent design patterns [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34711:
URL: https://github.com/apache/superset/pull/34711#issuecomment-3201847227
@mistercrunch Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/17078881996). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
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] feat: refactor modals to use consistent design patterns [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34711:
URL: https://github.com/apache/superset/pull/34711#issuecomment-3201879419
@mistercrunch Ephemeral environment spinning up at http://54.185.17.23:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
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] feat: refactor modals to use consistent design patterns [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on PR #34711:
URL: https://github.com/apache/superset/pull/34711#issuecomment-3190354316
Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment `/korbit-review`.
> Your admin can change your review schedule in the [Korbit Console](https://app.korbit.ai/settings)
[](71e546d9-8e9b-49d2-a84a-e5669c2eaab1)
--
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] feat: refactor modals to use consistent design patterns [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34711:
URL: https://github.com/apache/superset/pull/34711#issuecomment-3208662674
@mistercrunch Ephemeral environment spinning up at http://44.252.42.77:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
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] feat: refactor modals to use consistent design patterns [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34711:
URL: https://github.com/apache/superset/pull/34711#issuecomment-3208642434
@mistercrunch Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/17114718674). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
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] feat: refactor modals to use consistent design patterns [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch merged PR #34711:
URL: https://github.com/apache/superset/pull/34711
--
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] feat: refactor modals to use consistent design patterns [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34711:
URL: https://github.com/apache/superset/pull/34711#issuecomment-3204244353
@yousoph Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/17089435143). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
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] feat: refactor modals to use consistent design patterns [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34711:
URL: https://github.com/apache/superset/pull/34711#issuecomment-3208267468
@mistercrunch Ephemeral environment spinning up at http://34.223.203.180:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
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] feat: refactor modals to use consistent design patterns [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34711:
URL: https://github.com/apache/superset/pull/34711#issuecomment-3201354788
@mistercrunch Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/17075281021). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
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] feat: refactor modals to use consistent design patterns [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34711:
URL: https://github.com/apache/superset/pull/34711#issuecomment-3204250229
@yousoph Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/17089488785). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
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] feat: refactor modals to use consistent design patterns [superset]
Posted by "yousoph (via GitHub)" <gi...@apache.org>.
yousoph commented on PR #34711:
URL: https://github.com/apache/superset/pull/34711#issuecomment-3204234517
1. We don't always use the green check mark in the modals - we do that in Alerts & Reports but not in DB connection advanced settings (where everything is not optional). @kasiazjc in the like dashboard properties where some sections have required fields and others don't, should we only show the green check in sections with required sections?
2. Dashboard Name hint text needs updating, or just remove it to match the Chart name field
<img width="542" height="426" alt="image" src="https://github.com/user-attachments/assets/99dbe7f3-364e-4f33-87ba-dfbf4188e63d" />
3. The modal is shrinking in height when all sections are collapsed - it should stay the same height. Example:
<img width="523" height="872" alt="image" src="https://github.com/user-attachments/assets/a7831293-8c87-499c-ad63-f258920027b4" />
--
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] feat: refactor modals to use consistent design patterns [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on PR #34711:
URL: https://github.com/apache/superset/pull/34711#issuecomment-3201354831
Korbit doesn't automatically review large (3000+ lines changed) pull requests such as this one. If you want me to review anyway, use `/korbit-review`.
[](e1969036-2aee-46b4-84ab-a157040fba75)
--
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] feat: refactor modals to use consistent design patterns [superset]
Posted by "kasiazjc (via GitHub)" <gi...@apache.org>.
kasiazjc commented on PR #34711:
URL: https://github.com/apache/superset/pull/34711#issuecomment-3206263542
> 1. We don't always use the green check mark in the modals - we do that in Alerts & Reports but not in DB connection advanced settings (where everything is not optional). @kasiazjc in the like dashboard properties where some sections have required fields and others don't, should we only show the green check in sections with required sections?
I would use check/x for sections, it will be easier to spot what is missing. It would be great if we could add a rule, that all required fields should have red `*` next to the label across the app - not sure if it is something we can do easily @mistercrunch ?
> 4. Dashboard Name hint text needs updating, or just remove it to match the Chart name field
>
> <img alt="image" width="542" height="426" src="https://private-user-images.githubusercontent.com/10627051/479825735-99dbe7f3-364e-4f33-87ba-dfbf4188e63d.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NTU2OTUxMTMsIm5iZiI6MTc1NTY5NDgxMywicGF0aCI6Ii8xMDYyNzA1MS80Nzk4MjU3MzUtOTlkYmU3ZjMtMzY0ZS00ZjMzLTg3YmEtZGZiZjQxODhlNjNkLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTA4MjAlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwODIwVDEzMDAxM1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTRhNjg3MjQxMWJmZGQzN2VjZGZlMDc4MWQxYjkxNzQ2NzcyNGZhYTgzYzYwZTk3MjQwOTc5YzllNDkwMjUyOWImWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.dYN9MlnkRq59mKc1M9FnJLEnPd1Ea50kLYLixzlvj1g">
In general I would avoid using metadata for basic hints - can we move it to the field as placeholder? @mistercrunch
One more note from me (for now) is that errors should be shown in metadata in red under the field. This warning should show up next to the label.
<img width="858" height="216" alt="image" src="https://github.com/user-attachments/assets/4392ba2c-8911-4900-a963-08894b5ec4b4" />
--
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] feat: refactor modals to use consistent design patterns [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34711:
URL: https://github.com/apache/superset/pull/34711#issuecomment-3208247716
@mistercrunch Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/17111686012). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
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