[8549] Quick create enhancements#4803
Conversation
…include PathSelector
…efaultPaths based on item type
|
No actionable comments were generated in the recent review. 🎉 WalkthroughAdds a path-selection workflow to PathWithMacroCreator, makes Quick Create archetype-aware with grouped items and content-type-derived default paths, and updates the configuration epic to refresh the quick-create list when content-type form-definition XML changes. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant PWMC as PathWithMacroCreator
participant Store as Redux Store
participant Dialog as PathSelectionDialog
User->>PWMC: click "open path selector" (search icon)
PWMC->>Store: dispatch pushDialog(PathSelectionDialog)
Store-->>Dialog: show PathSelectionDialog
User->>Dialog: select path + click OK
Dialog->>Store: onOk(selectedPath)
Store->>PWMC: popDialog + deliver selectedPath
PWMC->>PWMC: set form value to selectedPath
sequenceDiagram
participant User as User
participant QC as QuickCreate
participant Store as Redux Store
participant Epic as Configuration Epic
participant Form as NewContentFormDialog
User->>QC: open quick create, choose grouped item
QC->>QC: compute defaultPath from contentType.type
QC->>Store: open NewContentFormDialog (with defaultPath)
User->>Form: save content-type/form-definition
Form->>Store: emit system event / contentTypeUpdated
Store->>Epic: handleSystemEvent -> returns fetchQuickCreateList()
Epic->>Store: dispatch fetchQuickCreateList
Store->>QC: update quickCreateItems
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@ui/app/src/components/ContentTypeManagement/controls/PathWithMacroCreator.tsx`:
- Around line 157-162: The IconButton in PathWithMacroCreator.tsx uses an
incorrect aria-label ("Maximize"); update its aria-label to match the
tooltip/FormattedMessage (e.g., "Select path") to improve accessibility and
consistency—locate the IconButton that calls onOpenPathSelectionDialog() within
the PathWithMacroCreator component and replace aria-label="Maximize" with
aria-label matching the tooltip text.
🧹 Nitpick comments (5)
ui/app/src/components/QuickCreate/QuickCreate.tsx (3)
178-201: Prefer stable keys over index-based keys for the grouped items.Line 179 uses
indexas the key for archetype groups, and Line 193 usesindexfor items within each group. Sincearchetype(the group label) is unique per group and each item likely has a uniquecontentTypeId, these are better candidates for keys. Index-based keys can cause subtle rendering issues if the list order or composition changes.♻️ Suggested key improvements
- {Object.keys(groupedItems).map((archetype, index) => ( - <Box key={index}> + {Object.keys(groupedItems).map((archetype) => ( + <Box key={archetype}> <Typography ... </Typography> - {groupedItems[archetype].map((item, index) => ( + {groupedItems[archetype].map((item) => ( <MenuItem - key={index} + key={item.contentTypeId} onClick={() => onItemSelected(item)}
364-372: Missing messageidfor the "Other" fallback label.Line 367 uses
formatMessage({ defaultMessage: 'Other' })without a messageid. While this works at runtime with react-intl v8, it won't be picked up by message extraction tools and is inconsistent with the rest of the codebase which usesdefineMessages/defineMessagewith explicit ids. Consider defining this message alongside the othertranslationsat the top of the file.♻️ Suggested approach
Add to the
translationsobject at the top of the file:otherArchetype: { id: 'quickCreate.otherArchetype', defaultMessage: 'Other' }Then reference it:
- const archetypeLabel = archetype?.name || formatMessage({ defaultMessage: 'Other' }); + const archetypeLabel = archetype?.name || formatMessage(translations.otherArchetype);
111-123: Duplicated default path logic withPathWithMacroCreator.The
type === 'page' ? '/site/website/' : '/site/components/'pattern on lines 114–115 is repeated inPathWithMacroCreator.tsx(line 74). Consider extracting this into a shared utility to keep the path defaults in one place.ui/app/src/components/ContentTypeManagement/controls/PathWithMacroCreator.tsx (2)
148-153: Auto-populating on focus may cause unintended form dirty state.When the user focuses the empty input (e.g., tabbing through the form),
setValue(defaultPath)fires, which likely marks the form as dirty/changed without explicit user intent. Theplaceholderalready communicates the default. Consider deferring the auto-population to a later user action (e.g., on blur if still empty, or only when the form is submitted) to avoid accidental state changes.
107-125: Consider initializing the path selection dialog at the current value or default path.
initialPathis hardcoded to'/site'. If the input already has a value (or has thedefaultPath), it would be more ergonomic to open the dialog at that location so the user doesn't need to navigate from the root every time.♻️ Suggested improvement
const onOpenPathSelectionDialog = () => { const id = nanoid(); dispatch( pushDialog({ id, component: 'craftercms.components.PathSelectionDialog', props: { rootPath: '/site', allowSwitchingRootPath: false, - initialPath: '/site', + initialPath: value || defaultPath, onClose: () => dispatch(popDialog({ id })), onOk: ({ path }) => { setValue(path); dispatch(popDialog({ id })); } } }) ); };
ui/app/src/components/ContentTypeManagement/controls/PathWithMacroCreator.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@ui/app/src/components/ContentTypeManagement/controls/PathWithMacroCreator.tsx`:
- Around line 149-153: The current onFocus handler in PathWithMacroCreator calls
setValue(defaultPath) which marks the form dirty when the user simply focuses
the field; change this so focus does not commit a value: remove the
setValue(defaultPath) from the onFocus, use the defaultPath as the input's
placeholder prop instead, and only programmatically set the value either when
the user begins typing (e.g., in the input's onChange/onKeyDown if the current
value is empty) or during form submission if the field is still empty; update
references in PathWithMacroCreator (remove setValue usage in the onFocus
handler, add placeholder={defaultPath} and add logic in the input change/submit
handlers to apply defaultPath when appropriate).
- Around line 72-74: The code reads originalValues.type without a type guard;
update the access used in PathWithMacroCreator to match the project's pattern by
either casting originalValues.type as string or guarding it (e.g.,
originalValues?.type) before comparing; specifically modify the
useStableFormContext()/originalValues usage that sets defaultPath so the
expression becomes type-safe (e.g., treat originalValues.type as string or check
for undefined) and avoid silently falling back without an explicit cast/check.
🧹 Nitpick comments (3)
ui/app/src/components/ContentTypeManagement/controls/PathWithMacroCreator.tsx (3)
107-125: Path selection replaces the entire input value — verify this is intentional.When the user picks a path from
PathSelectionDialog,onOkcallssetValue(path), which overwrites whatever was previously in the input (including any macros the user may have already appended). If the intent is to use this as a base path before adding macros, this is fine, but it could surprise users who added macros first and then decide to change the base path.Also, minor: the
onClosecallback doesn't calldialogState.onClose— it only pops the dialog stack. This seems correct since this dialog is managed via the Redux dialog stack (notdialogState), just noting for clarity.
157-165: Aria-label fix looks good; simplify theonClickhandler.The
aria-labelnow correctly matches the tooltip — nice fix. Minor nit: the wrapping arrow function is unnecessary sinceonOpenPathSelectionDialogtakes no arguments.Suggested simplification
- <IconButton - aria-label={formatMessage({ defaultMessage: 'Select path' })} - onClick={() => onOpenPathSelectionDialog()} - > + <IconButton + aria-label={formatMessage({ defaultMessage: 'Select path' })} + onClick={onOpenPathSelectionDialog} + >
197-205: UnusedPopover— appears to be dead code.
anchorElis initialized toundefinedandsetAnchorElis never called in this component, so thisPopovercan never open. If this is leftover from a previous implementation, consider removing it along with theanchorElstate on Line 77.#!/bin/bash # Verify setAnchorEl is never called with a truthy value rg -n "setAnchorEl" --type=ts -g '*PathWithMacroCreator*'
ui/app/src/components/ContentTypeManagement/controls/PathWithMacroCreator.tsx
Show resolved
Hide resolved
ui/app/src/components/ContentTypeManagement/controls/PathWithMacroCreator.tsx
Show resolved
Hide resolved
ui/app/src/components/ContentTypeManagement/controls/PathWithMacroCreator.tsx
Outdated
Show resolved
Hide resolved
2 participants
craftercms/craftercms#8549
Summary by CodeRabbit
New Features
Improvements