fix(congregation): hint instead of empty fields #3425
Conversation
|
Task linked: CU-86c1nvavr [FIX] Hint instead of empty fields |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request enhances the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/features/congregation/settings/meeting_settings/midweek/auxiliary_classroom/index.tsx (1)
72-88: Consider standardizing the padding values.The implementation looks good and achieves the goal of showing a helpful hint message. However, consider using Material-UI's standard spacing units for consistency.
- padding: '0px 8px 0px 8px', + padding: theme.spacing(0, 1),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/locales/en/congregation.jsonis excluded by!**/*.json
📒 Files selected for processing (2)
src/components/select/index.tsx(1 hunks)src/features/congregation/settings/meeting_settings/midweek/auxiliary_classroom/index.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Code QL
- GitHub Check: Summary
🔇 Additional comments (2)
src/components/select/index.tsx (1)
67-67: LGTM! Clean implementation of style customization.The spread operator is correctly placed to allow custom styles to override the default styles while maintaining the base styling.
src/features/congregation/settings/meeting_settings/midweek/auxiliary_classroom/index.tsx (1)
61-70: LGTM! Good handling of menu width.The conditional width adjustment provides a better user experience by maintaining a consistent minimum width when empty.
…uk' of https://github.com/FussuChalice/organized-app into CU-86c1nvavr_FIX-Hint-instead-of-empty-fields_Max-Makaluk
There was a problem hiding this comment.
@ux-git, @FussuChalice: I am not really sure where you intended to show this hint. Currently it is now showing anywhere. Also based on the new text added, I do no think showing such a long text in a select field would be very efficient. Surely, the text won’t fit. Also, I can’t test it yet but will this display that fallback for every instance of the select field where there are no children?
|
The correct approach would be to render the default option inside the instance of the Select component we want to edit, and assign it an empty value. An example of this can be found in the MUI docs. |
|
@rhahao yes, the intention is to show the placeholder for person select dropdown when there are no children/results: 
This will ensure that user gets the feedback instead of the half-empty list (that might look more like a bug) and the hint how to resolve the issue of having no results. The goal is to have this |
|
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
Checklist: