fix: show required marker on legacy select widgets - #41927
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (9)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. WalkthroughSelect, multi-select, and tree-select widgets now accept an optional ChangesRequired-state wiring for select widgets
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change makes required markers render on the affected legacy select widgets without altering WDS widgets or validation behavior. No actionable merge-blocking risk remains after normal checks and review. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Your free Security trial is over. An organization admin can activate Security or dismiss this notice. Comment |
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
|
@miachillgood - this has been marked as stale as there are checks failed ❌ Failed checks (1 warning)Check name | Status | Explanation | Resolution -- | -- | -- | -- Docstring Coverage | |
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
|
@appsmith-bot who can help with this? |
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/29471161982. |
|
Deploy-Preview-URL: https://ce-41927.dp.appsmith.com |
|
@miachillgood @j0hnby You can test the feature using the following link: ce-41927.dp.appsmith.com |
|
@miachillgood please fix the linting errors |
|
Thanks for re-testing. I believe the lint issue was addressed in I also checked the remaining If there is anything else needed on the code side, please let me know and I can update it. |
|
Thanks @miachillgood and @j0hnby . Allow me some time to get back to you about the test suite failure |
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
|
Following up on this one since the stale bot fired again. From my side, I don't see any new code-side feedback to address, and the remaining blocker still looks like the PR automation If there is anything else you want me to update in the PR, I can do that. Otherwise, I would appreciate a quick nudge once the test-suite/automation side is unblocked. |
|
Hi @miachillgood Thanks for the heads up! We don't need any changes from your side for now. We're currently working on a fix to get the broken pipeline for community contributors working again. We'll let you know as soon as it's unblocked. Thanks for your patience! |
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
|
This PR has been closed because of inactivity. |
|
This was closed automatically for inactivity while the last maintainer update said no code changes were needed and the community-contributor pipeline fix was still in progress. The deploy preview was tested successfully in the thread. I tried to reopen it, but GitHub did not allow the reopen action. Could a maintainer please reopen it when the contributor pipeline is ready? I am happy to address any new code-side feedback if something else is needed. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/approve-ci |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/33405210871. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/33405210871. |


Summary
isRequiredtoLabelWithTooltipfor the legacy select widgets covered by [Bug]: Required marker (*) not displayed on required select widgets #41734isRequiredprop throughSelectWidget,MultiSelectWidget,MultiSelectWidgetV2,SingleSelectTreeWidget, andMultiSelectTreeWidgetRoot cause
These widgets already expose and use
isRequiredfor validation, but the prop was not forwarded to the label component, so the required marker never rendered in the label UI.Testing
git diff --checkyarn eslintin this minimal checkout because the workspace does not include the local install state (Couldn't find the node_modules state file)Closes #41734
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Caution
If you modify the content in this section, you are likely to disrupt the CI result for your PR.
Summary by CodeRabbit
New Features
Bug Fixes