feat(FR-3441): simplify the folder creation form - #9398
Open
nowgnuesLee wants to merge 6 commits into
Open
Conversation
Apply the immediately-actionable items from the FR-3441 folder-creation UX review: - Move the folder name field to the top of the form. On the super-admin Data page the target project selector stays above it, and the divider under it now carries the same 24px bottom margin form items have, so the gap below it is no longer missing. - Remove the Type (user/project) radio group. The page context already decides the ownership, so `folderType` now derives it directly: 'project'/'model_project' create project folders, unset creates a user folder. The unused `allowCreateProjectFolder` prop and the `list_allowed_types` REST query (which only fed the radio options) go with it. The model-store gating that lived on the type field moves to `usage_mode`, and the in-modal project selector revalidates it on selection. - Update the admin data page banner copy in all 21 locales from "only project folders can be created" to "folders are created as project folders", matching the removed selector. - Remove the Reset button from the modal footer — the form has too few fields to warrant it. - Hide options that cannot be selected instead of disabling them: the automount usage mode on project-folder pages, and the Read & Write permission when a model project folder forces read-only (FR-1290). Selecting the Models usage mode on a project-folder page now coerces the permission value to 'ro' so the hidden option can't linger as the field value. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01NZrbeByLaXG46gcmGXAAXY
github-actions
Bot
added
area:ux
nowgnuesLee
added a commit
that referenced
this pull request
Sep 2, 2026
nowgnuesLee
added a commit
that referenced
this pull request
Sep 2, 2026
nowgnuesLee
added a commit
that referenced
this pull request
Sep 2, 2026
nowgnuesLee
added a commit
that referenced
this pull request
Sep 2, 2026
Contributor
Coverage Report for react-coverage (./react)
File Coverage
|
||||||||||||||||||||||||||||||||||||||
…osen The first cut hid the Read & Write option and coerced the value to 'ro' whenever Models was selected on a project-folder page, even when the target project is not the model-store project — where the combination is invalid anyway and the permission field should stay as it always was. Gate the FR-1290 read-only forcing on the effective project actually being the model-store project, matching the pre-FR-3441 model-store distinction, and apply the same coercion when the in-modal project selector lands on the model-store project while Models is already selected. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01NZrbeByLaXG46gcmGXAAXY
nowgnuesLee
added a commit
that referenced
this pull request
Sep 3, 2026
nowgnuesLee
added a commit
that referenced
this pull request
Sep 3, 2026
Revert the hide-and-coerce treatment of the Read & Write option entirely: the rw radio is shown again and disabled with the warning tooltip whenever Models is selected on a project-folder page, exactly as before this PR. The FR-3441 scope covers only the field order, the type radio removal, the banner copy, the reset button, and the automount option; the mount-permission model-store handling is out of scope. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01NZrbeByLaXG46gcmGXAAXY
nowgnuesLee
added a commit
that referenced
this pull request
Sep 3, 2026
nowgnuesLee
added a commit
that referenced
this pull request
Sep 3, 2026
…fallback The target-project selector's fallback was a bare Skeleton, which doesn't read as a select; render a disabled loading BAISelect instead, matching the ImportHuggingFaceModelForm pattern. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01NZrbeByLaXG46gcmGXAAXY
Apply .claude/rules/comment-density.md to the file this PR already touches: drop the PILOT-DECISION blocks, ticket-numbered migration narration, and the stale footer marginSM note; compress the remaining blocks to the constraint plus a pointer (QA-FINDINGS Q-26, FR-1290, ADR-0001, FR-3700 all kept as one-to-three-line notes). Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01NZrbeByLaXG46gcmGXAAXY
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Project validation can use stale state, cluster ownership capabilities are bypassed, and existing E2E tests still assert the removed controls.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Simplifies folder creation by deriving ownership from page context and streamlining the modal.
Changes:
- Removes folder-type selection, reset action, and allowed-types query.
- Reorders fields and updates project/model validation behavior.
- Updates ownership banners across 21 locales.
File summaries
| File | Description |
|---|---|
react/src/components/FolderCreateModalV2.tsx |
Simplifies form and derives folder ownership. |
resources/i18n/en.json |
Updates English banners. |
resources/i18n/ko.json |
Updates Korean banners. |
resources/i18n/de.json |
Updates German banners. |
resources/i18n/el.json |
Updates Greek banners. |
resources/i18n/es.json |
Updates Spanish banners. |
resources/i18n/fi.json |
Updates Finnish banners. |
resources/i18n/fr.json |
Updates French banners. |
resources/i18n/id.json |
Updates Indonesian banners. |
resources/i18n/it.json |
Updates Italian banners. |
resources/i18n/ja.json |
Updates Japanese banners. |
resources/i18n/mn.json |
Updates Mongolian banners. |
resources/i18n/ms.json |
Updates Malay banners. |
resources/i18n/pl.json |
Updates Polish banners. |
resources/i18n/pt.json |
Updates Portuguese banners. |
resources/i18n/pt-BR.json |
Updates Brazilian Portuguese banners. |
resources/i18n/ru.json |
Updates Russian banners. |
resources/i18n/th.json |
Updates Thai banners. |
resources/i18n/tr.json |
Updates Turkish banners. |
resources/i18n/vi.json |
Updates Vietnamese banners. |
resources/i18n/zh-CN.json |
Updates Simplified Chinese banners. |
resources/i18n/zh-TW.json |
Updates Traditional Chinese banners. |
Review details
- Files reviewed: 22/22 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
…2e contract Two of the three Copilot findings on PR #9398 are real and fixed here: - The in-modal project selector validated `usage_mode` synchronously after `setSelectedProject`, so the rule from the previous render ran against the previous `effectiveProject` — picking the model-store project while Models was selected left a false error. The revalidation moved to an effect keyed on `selectedProject`, which runs after the new rule closure is registered. Live-verified: no project + Models shows the error, choosing model-store clears it, switching back to another project restores it. - The Playwright suite still asserted the removed Type radios and Reset button. `vfolder-type-selection.spec.ts` now asserts the page-derived ownership contract (no Type row on either page, Auto Mount hidden on /project-data, the model-store gate on Usage Mode, Read & Write disabled for model project folders), `FolderCreationModal` drops the Type/Reset accessors for an `expectTypeFormItemHidden()` assertion, and `createVFolderAndVerify` rejects a `type: 'project'` argument instead of clicking a radio that no longer exists. The third finding (re-adding `list_allowed_types()` to gate submission) is answered on the thread: the removed query only filtered radio options and never gated submission, so this PR does not regress that behavior. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01NZrbeByLaXG46gcmGXAAXY
nowgnuesLee
marked this pull request as ready for review
September 3, 2026 02:35
agatha197
requested changes
Sep 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #8531 (FR-3441)
applied test server: http://fr-3441.seungwon.10-82-0-159.sslip.io/
Applies the immediately-actionable items from the FR-3441 folder-creation UX review to
FolderCreateModalV2.Changes
folderTypenow derives it:'project'/'model_project'→ project folder, unset → user folder. The unusedallowCreateProjectFolderprop and thelist_allowed_typesREST query (which only fed the radio options) are removed with it. The model-store gating that lived on the type field moved tousage_mode, and the in-modal project selector revalidates it on selection.Screenshots
Verification
bash scripts/verify.sh→=== ALL PASS ===FolderCreateModalV2.test.tsx5/5,ImportArtifactRevisionToFolderModal.test.tsx+DeploymentAddRevisionModal.test.tsx8/8 pass🤖 Generated with Claude Code