Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add layout API documentation. #52673

Merged
merged 4 commits into from
Jul 18, 2023
Merged

Add layout API documentation. #52673

merged 4 commits into from
Jul 18, 2023

Conversation

tellthemachines
Copy link
Contributor

What?

Adds API docs for layout support which was stabilised in #51434.

Testing Instructions

Read and check that everything makes sense.

Testing Instructions for Keyboard

Screenshots or screencast

tellthemachines
tellthemachines commented Jul 17, 2023
View reviewed changes
docs/reference-guides/block-api/block-supports.md

Determines display of layout controls in the block sidebar. If set to false, layout controls will be hidden.

### layout.allowInheriting
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this property still makes sense. It's being used incorrectly across several core blocks (it doesn't do anything for blocks with flex type) and if we really don't want the content width toggle to display it's easier to just switch off allowEditing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it probably doesn't make much sense any more. Back when inheriting was a feature within a single layout type it probably did, but now we have two distinct layout types flow and constrained, so it probably doesn't make sense on its own.

I think it highlights that it was a good decision splitting the complex behaviour of inherit out into two separate layout types!

tellthemachines
tellthemachines commented Jul 17, 2023
View reviewed changes
schemas/json/block.json Outdated
@@ -336,6 +336,59 @@
"description": "By default, all blocks will appear in the inserter, block transforms menu, Style Book, etc. To hide a block from all parts of the user interface so that it can only be inserted programmatically, set inserter to false.",
"default": true
},
"layout": {
"type": [ "boolean", "object" ],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sooo we have a bit of a problem here, in that our schema validation doesn't allow properties to have multiple types like this 😂

Cc @ajlende who added strict validation in #44423.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use "oneOf"? Something like "settingsPropertiesTypography.fluid" has:

"oneOf": [
	{ "type": "boolean" },
	{ "type": "object" }
]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooooooh

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated; still seeing test failures locally though

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems to have fixed the issue on CI! yay 🎉

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweeeeeet! 🌮

ramonjd
ramonjd reviewed Jul 17, 2023
View reviewed changes
docs/reference-guides/block-api/block-supports.md
- Type: `Object`
- Default value: null

Allows setting the `type` property to define what layout type is default for the block, and also default values for any properties inherent to that layout type, e.g., for a `flex` layout, a default value can be set for `flexWrap`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also default values for any properties inherent to that layout type, e.g., for a flex layout, a default value can be set for flexWrap.

No biggie, but just curious about whether we should provide illustrative examples since theme.json doesn't cover any of the possible extended values?

			"default": {
				"type": "flex",
				"flexWrap": "nowrap"
			}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm good point perhaps we should add all the possible values to the schema, or they might erroneously be flagged as invalid

@andrewserong andrewserong added the [Type] Developer Documentation Documentation for developers label Jul 17, 2023
tellthemachines
tellthemachines commented Jul 17, 2023
View reviewed changes
schemas/json/block.json
"default": {
"type": "object",
"description": "Allows setting the `type` property to define what layout type is default for the block, and also default values for any properties inherent to that layout type, e.g., for a `flex` layout, a default value can be set for `flexWrap`.",
"properties": {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I think these are all the possible layout attributes below.

@github-actions
Copy link

Flaky tests detected in ae5b43d.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5572463234
📝 Reported issues:

ramonjd
ramonjd approved these changes Jul 17, 2023
View reviewed changes
Copy link
Member

@ramonjd ramonjd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates!

andrewserong
andrewserong approved these changes Jul 17, 2023
View reviewed changes
Copy link
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, too! While allowInheriting probably doesn't quite make sense as a flag any more, the description of it here reads well for the flow layout, and we can likely look into adding / deprecating subproperties in later versions. For now this PR provides the much needed documentation for the current state of things 👍

@tellthemachines
Copy link
Contributor Author

Thanks for the reviews and suggestions folks!

@tellthemachines tellthemachines merged commit 24ecc09 into trunk Jul 18, 2023
@tellthemachines tellthemachines deleted the add/layout-api-docs branch July 18, 2023 00:59
@github-actions github-actions bot added this to the Gutenberg 16.3 milestone Jul 18, 2023
@tellthemachines tellthemachines self-assigned this Jul 18, 2023
westonruter added a commit that referenced this pull request Jul 18, 2023
@westonruter
Merge branch 'trunk' into add/defer-script-loading-strategy
7fe5534 
* trunk: (36 commits)
  Use `_get_block_template_file` function and set $area variable. (#52708)
  Change Delete page menu item to Move to trash. (#52641)
  Search block: Enqueue view script through block.json (#52552)
  Patterns: fix bug with Create Patterns menu not showing in site editor page editing (#52671)
  Github workflow: add a PHP backport changes action (#52096)
  Add layout API documentation. (#52673)
  Show uncategorized patterns on the Editor > Patterns page (#52633)
  Patterns: Remove `reusable` text from menu once rename hint has been dismissed (#52664)
  Update locked pattern tooltips (#52497)
  Rich Text/Footnotes: fix getRichTextValues for useInnerBlocksProps.save (#52682)
  Use posts instead of template parts for navigation color tests (#52654)
  Site Editor: Don't allow creating template part on the Patterns page for non-block themes (#52656)
  Site Editor: Fix incorrect 'useSelect' usage (#52683)
  Update issue gardening automation with new label (#52173)
  i18n: Make the tab labels of `ColorGradientSettingsDropdown` component translatable (#52669)
  Post Content link color should not be applied to placeholder component links (#52367)
  [Mobile] Update toolbar icons and colors (#52336)
  Avoid copying global style presets via the styles compatibility hook (#52640)
  Show warning on removal of Post Template block in the site editor. (#52666)
  Backport tools: sort PRs to be cherry picked by merged/closed date (#52667)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewserong andrewserong andrewserong approved these changes

@ramonjd ramonjd ramonjd approved these changes

@ajlende ajlende Awaiting requested review from ajlende

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

@ryanwelcher ryanwelcher Awaiting requested review from ryanwelcher

@juanmaguitar juanmaguitar Awaiting requested review from juanmaguitar juanmaguitar is a code owner

@fabiankaegy fabiankaegy Awaiting requested review from fabiankaegy fabiankaegy is a code owner

@ndiego ndiego Awaiting requested review from ndiego ndiego is a code owner

Assignees

@tellthemachines tellthemachines

Labels
[Type] Developer Documentation Documentation for developers
Projects
None yet
Milestone
Gutenberg 16.3
Development

Successfully merging this pull request may close these issues.
3 participants
@tellthemachines @ramonjd @andrewserong