-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Block Grid/List: Skip workspace modal for blocks with no properties #21316
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@copilot yes please let me see how this looks in code. |
Co-authored-by: nielslyngsoe <[email protected]>
Co-authored-by: nielslyngsoe <[email protected]>
Co-authored-by: nielslyngsoe <[email protected]>
I've implemented the solution in commits 529913c, 4e8a040, and 33a0b23. The implementation adds a check for Block Grid changes:
Block List changes:
Both implementations include error handling and use a safe default ( The solution is ready for testing with blocks that have no properties (only areas). |
|
|
||
| // Only append the workspace modal path if the block type has properties | ||
| // If it has no properties (only areas), skip the modal and create directly | ||
| if (hasProperties === false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot I dont think this works. It would need to call a method, when its not suppose to open a modal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're absolutely right! I've fixed this in commit c318a42.
The Block Grid now properly creates blocks inline when they have no properties:
- Context returns
undefined(not a catalogue path) when block has no properties - Element detects undefined path and calls
#createBlockInline()method - Method uses
context.create()andcontext.insert()to create the block directly - Click handler prevents default navigation when path is undefined
This matches the Block List implementation where blocks are created inline without opening any modal.
Co-authored-by: nielslyngsoe <[email protected]>
|
I'm not happy with this. |
When only one block type is available and it contains no content properties (only areas), the current implementation opens an empty workspace modal. This creates unnecessary friction in the UX flow.
Solution Implemented
The implementation checks
contentTypeHasPropertiesfrom the block's structure before deciding whether to open the workspace modal:structure.contentTypeHasPropertiesChanges Made
Block Grid (
block-grid-entries.context.tsandblock-grid-entries.element.ts)DEFAULT_BLOCK_HAS_PROPERTIESconstant for safe default behavior#singleBlockTypeHasPropertiesstate to track if single block type has properties#setupSingleBlockTypePropertyCheck()method with promise-based async handling and error recoverygetPathForCreateBlock()to returnundefinedwhenhasProperties === false(triggers inline creation)_allowedBlockTypesstate to element to track available block types#createBlockInline()method that callscontext.create()andcontext.insert()to create blocks directlyundefined(block has no properties)#createBlockInline()to create the block immediatelyBlock List (
property-editor-ui-block-list.element.ts)DEFAULT_BLOCK_HAS_PROPERTIESconstant for safe default behavior_singleBlockTypeHasPropertiesstatecontentTypeHasPropertiesasynchronously#getPathForCreateBlock()to returnundefinedwhen block has no properties (triggers inline creation)Technical Details
.then()pattern instead of async callbacks to avoid race conditions.catch()error handling for robust fallback behaviorundefinedpath → detect in click handler → create inline via context methodsThis provides a better UX by avoiding empty modals for blocks that only define areas, while maintaining backward compatibility.
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.