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.

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

Block Support: Separate opt in for font style and weight options #26844

Conversation

aaronrobertshaw
Copy link
Contributor

Description

Updates the recently added font appearance block support to separate the style and weight aspects in terms of opting in via block.json.

This includes:

  • Updating the detection of support opt-in within typography tools
  • Updating the UI control to handle the different opt-in situations
  • Adding translation for the combined UI option labels allowing order to be switched if appropriate for a locale
  • Extracting concerns of how the selections are stored within style attributes from the UI control

How has this been tested?

Manually.

Testing Instructions

  1. Check out this PR
  2. Edit a post adding a navigation block
  3. Select the navigation block and ensure the ability to select a font style and weight combination still works
  4. Update packages/block-library/src/navigation/block.json toggling __experimentalFontStyle off in the supports array
  5. Reload the editor( you may need to restart dev env to pickup the changed block.json )
  6. Confirm the navigation block displays a "Font weight" option instead of the previous "Appearance" option and it works
  7. Update the block.json again toggling on __experimentalFontStyle and toggling off __experimentalFontWeight
  8. Confirm the navigation block displays a "Font style" option and it works
  9. Finally, test removing or toggling off both options and confirm there are no errors and no font style/weight option is displayed.

Screenshots

Style & Weight Style Only Weight Only
FontStyleWeight1 FontStyleWeight2 FontStyleWeight3

Types of changes

Breaking change.

This changes the block support flags for the font style/weight feature that was recently merged but not yet part of a release.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@apeatling
Copy link
Contributor

Tested and confirmed this is working as expected in the instructions. 👍

Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

From the functional point view everything is working as expected 👍

packages/block-editor/src/hooks/font-appearance.js Outdated Show resolved Hide resolved
lib/global-styles.php Outdated Show resolved Hide resolved
( { slug } ) => slug === newStyles.fontWeight
);
const newFontWeight = presetWeight
? `var:preset|font-weight|${ presetWeight.slug }`
Copy link
Member

Choose a reason for hiding this comment

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

Not directly related to this PR but I left a comment regarding the current preset usage on fontStyle and fontWeight PR's.
These things are breaking change and may affect block markup so I guess it is something we should look at and decide as soon as possible
#26444 (review).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been updated to have a separate value property in the presets.

@oandregal oandregal mentioned this pull request Nov 18, 2020
82 tasks
@apeatling apeatling self-requested a review November 18, 2020 16:49
Updates the recently added font appearance block support to separate the style and weight aspects in terms of opting in via block.json. This required:

* Updating the detection of support opt in within typography tools
* Updating the UI control to handle the situation when only one option is opted into

This change also includes adding translation for the combined UI option labels and extracting concerns of how the selections are stored within style attributes from the UI control.
@aaronrobertshaw
Copy link
Contributor Author

This has now been rebased and updated to be compatible with recent introduction of WP_Theme_JSON

aristath added a commit to aristath/q that referenced this pull request Nov 23, 2020
@apeatling apeatling self-requested a review November 24, 2020 01:00
Copy link
Contributor

@apeatling apeatling left a comment

Choose a reason for hiding this comment

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

I ran through the testing instructions again after the recent changes, and can confirm everything worked as expected. 👍

@aaronrobertshaw aaronrobertshaw merged commit df2e65b into WordPress:master Nov 24, 2020
@github-actions github-actions bot added this to the Gutenberg 9.5 milestone Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants