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

Rename theme support for wide images to `align-wide`. #4153

Merged
merged 5 commits into from Jan 5, 2018

Conversation

Projects
None yet
5 participants
@getsource
Contributor

getsource commented Dec 22, 2017

Description

Changes theme support for wide images from being within the gutenberg theme support array with a wide-images bool to adding it with: add_theme_support( 'align-wide' );

Additionally, updates documentation and related JS variable names to match.

How Has This Been Tested?

This was tested by adding theme support for align-wide to Twenty Seventeen.
Caveat: The demo content still has a wide image included, and it displays this way whether or not align-wide is enabled in the theme. As far as I can tell, this was also the case with the previous name.

Types of changes

This is a breaking change for themes that have been added wide image support using the previously supported method. They would need to change to the new naming for the wide and full image controls to display.
Fixes: #4113

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code has proper inline documentation.
Show outdated Hide outdated docs/themes.md
Show outdated Hide outdated lib/client-assets.php

@youknowriad youknowriad requested review from mcsf, karmatosed, jasmussen and aduth Dec 22, 2017

@mcsf

This comment has been minimized.

Show comment
Hide comment
@mcsf

mcsf Jan 2, 2018

Contributor

This looks quite good so far! @getsource, let us know once you'd like another look (per #4153 (comment)).

Contributor

mcsf commented Jan 2, 2018

This looks quite good so far! @getsource, let us know once you'd like another look (per #4153 (comment)).

@aduth

This comment has been minimized.

Show comment
Hide comment
@aduth

aduth Jan 2, 2018

Member

Noting that this change is also proposed in the implementation of #4069 (see "Implementation notes").

Member

aduth commented Jan 2, 2018

Noting that this change is also proposed in the implementation of #4069 (see "Implementation notes").

@getsource

This comment has been minimized.

Show comment
Hide comment
@getsource

getsource Jan 3, 2018

Contributor

I think this is ready for another look. Added colors to the mix and moved the warning. We could throw both a warning and a _doing_it_wrong(), too. Open to either one.

Re: @aduth's comment, if this, or part of this, is best done in a different pull request or issue, I'm fine with that. Just let me know so that I don't continue here. :)

Contributor

getsource commented Jan 3, 2018

I think this is ready for another look. Added colors to the mix and moved the warning. We could throw both a warning and a _doing_it_wrong(), too. Open to either one.

Re: @aduth's comment, if this, or part of this, is best done in a different pull request or issue, I'm fine with that. Just let me know so that I don't continue here. :)

@youknowriad

Nice work 👍

Show outdated Hide outdated docs/themes.md

getsource added some commits Dec 22, 2017

Rename theme support for wide images to `align-wide`
Changes theme support for wide images from being within the `gutenberg`
theme support array with a `wide-images` bool to adding it with:
`add_theme_support( 'align-wide' );`

Additionally, updates documentation and variable names to match.

See: #4113
Add backcompat for `wide-images` in gutenberg array.
Includes backcompat for declaring wide image support
with `add_theme_support` and `wide-images` inside the gutenberg array.

Issues a JS console warning with a link to documentation if it
is not declared with `add_theme_support( 'align-wide' );`.
Resolve eslint errors.
- When warning about `wide-images` usage, use single quotes for formatting.
- Whitelist usage of `console` for `wide-images` warning.
Move alerts to PHP and move `colors` out of array.
- Moves the console warning to be generated on the server side
- Moves `colors` out of the array as well, with `editor-color-palette`
as a replacement theme support flag.
- Updates docs to reflect both of these.

@youknowriad youknowriad merged commit c9a809d into WordPress:master Jan 5, 2018

2 checks passed

codecov/project 39.2% remains the same compared to 55c5a79
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

BinaryMoon added a commit to BinaryMoon/granule that referenced this pull request Jan 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment