make some modals closable #8869
Conversation
📝 WalkthroughWalkthroughReplaces Ant Design Modal footer-hiding via CSS/props with footer={null} in dataset views, adjusts mask closability on several modals by removing maskClosable={false}, and tidies confirm dialog options. Updates _utils.less by removing .no-footer-modal, adding dragging helpers, and minor selector/clearfix tweaks. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
| display: "none", | ||
| }, | ||
| }} | ||
| footer={null} |
There was a problem hiding this comment.
| title: `Deleting an annotation layer makes its content and history inaccessible. ${fallbackLayerNote}This cannot be undone. Are you sure you want to delete this layer?`, | ||
| okText: `Yes, delete annotation layer “${readableAnnotationLayerName}”`, | ||
| cancelText: "Cancel", | ||
| maskClosable: true, |
There was a problem hiding this comment.
confirmAsync uses a Modal.confirm under the hood. The intent of a confirm is to force a user decision. Therefore, both these options are not part of the defaults. I think that is sensible and we should not change the default behavior.
There was a problem hiding this comment.
This file was auto-formatted while saving.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/javascripts/admin/dataset/dataset_add_view.tsx (1)
271-280: Post-upload modal still blocks backdrop-click close; also remove unused onOkThis PR aims to make modals closable by clicking the mask. Here,
maskClosable={false}prevents that. Also, withfooter={null}, there is no OK button, soonOkis dead code.Apply:
return ( <Modal open closable - maskClosable={false} + maskClosable footer={null} onCancel={() => setDatasetId("")} - onOk={() => setDatasetId("")} width={580} >frontend/stylesheets/_utils.less (1)
52-63: Fix first/last-child selectors to target children, not the container
&:first-child/&:last-childselect.centered-itemswhen it is the first/last child of its parent. The intent here appears to be targeting the first/last direct child inside.centered-items..centered-items { display: flex; flex-direction: row; justify-content: center; - &>* { + &>* { margin: 6px; } - &:first-child { - margin-left: 0px; - } - - &:last-child { - margin-right: 0px; - } + & > *:first-child { + margin-left: 0; + } + + & > *:last-child { + margin-right: 0; + } }
🧹 Nitpick comments (2)
frontend/javascripts/admin/dataset/dataset_upload_view.tsx (1)
491-497: Consider enabling mask close (or document why this is one of the 14 exceptions)You switched to
footer={null}(good), butmaskClosable={false}still blocks backdrop-click closing, which may be intended for an in-progress upload. If this modal is not one of the 14 deliberate exceptions, enable mask close. If it is an exception, add a brief code comment stating why.Proposed change (keeps
keyboard={false}to avoid accidental ESC closes, still routes throughonCancel→ confirm dialog):<Modal open={isUploading} keyboard={false} - maskClosable={false} + maskClosable footer={null} onCancel={this.cancelUpload} >If you intentionally keep it non-closable by mask, consider:
- maskClosable={false} + // Keep mask non-closable to force explicit confirmation via onCancel + maskClosable={false}frontend/javascripts/viewer/view/left-border-tabs/modals/add_volume_layer_modal.tsx (1)
230-232: Make the intention explicit: allow backdrop-click closeYou removed the explicit
maskClosable={false}. To avoid relying on library defaults and to align with the PR goal, setmaskClosableexplicitly.- <Modal title="Add Volume Annotation Layer" footer={null} width={500} onCancel={onCancel} open> + <Modal + title="Add Volume Annotation Layer" + footer={null} + width={500} + onCancel={onCancel} + open + maskClosable + >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
frontend/javascripts/admin/dataset/dataset_add_view.tsx(2 hunks)frontend/javascripts/admin/dataset/dataset_upload_view.tsx(1 hunks)frontend/javascripts/admin/user/experience_modal_view.tsx(0 hunks)frontend/javascripts/viewer/view/left-border-tabs/layer_settings_tab.tsx(0 hunks)frontend/javascripts/viewer/view/left-border-tabs/modals/add_volume_layer_modal.tsx(1 hunks)frontend/stylesheets/_utils.less(4 hunks)
💤 Files with no reviewable changes (2)
- frontend/javascripts/viewer/view/left-border-tabs/layer_settings_tab.tsx
- frontend/javascripts/admin/user/experience_modal_view.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-smoketest-push
- GitHub Check: backend-tests
- GitHub Check: frontend-tests
🔇 Additional comments (4)
frontend/javascripts/admin/dataset/dataset_add_view.tsx (1)
329-329: No-op formatting change — looks fineNothing to do here.
frontend/stylesheets/_utils.less (3)
75-80: Clearfix addition looks goodAdding
clearfix::afterwithclear: bothcomplements the existing::beforeand resolves float clearing reliably.
139-156: Dragging state utilities — LGTMThe
.grabbing/.is-draggingrules are scoped and should not affect normal interactions. Good addition.
52-55: No lingering.no-footer-modalreferences found
A project-wide search forno-footer-modalreturned no matches, confirming that the CSS helper can be safely removed.
This PR update some modals to:
There remained 14 modals that can not be closed by just clicking outside them in the background. I looked through the list and for those modals I agree that they should not be (accidentally) closable but rather the user should be forced to make a decision by clicking one the Ok/Cancel/etc buttons.
Issues:
(Please delete unneeded items, merge only when none are left open)
$PR_NUMBER.mdfile inunreleased_changesor use./tools/create-changelog-entry.py)