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

Update button block CSS and add class to link #3445

Merged
merged 7 commits into from Jan 2, 2018

Conversation

Projects
None yet
3 participants
@samikeijonen
Contributor

samikeijonen commented Nov 12, 2017

Description

This PR moves all styles (also inline styles) to link itself. Also adds class to link for easier styling in the editor. See issue #2907.

Also align center and right works now.

How Has This Been Tested?

  • Tested with default themes and random .org themes.
  • Run npm test without errors.
  • Tested alignment, background-color, and text color changes.

Screenshots (jpeg or gifs if applicable):

Types of changes

  • Moves styles to link itself.
  • Moves inline styles to link itself.
  • Add class wp-block-button-link to link. Note that I don't is this the correct way to add class.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows has proper inline documentation.
@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Nov 12, 2017

Codecov Report

Merging #3445 into master will decrease coverage by 2.21%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3445      +/-   ##
==========================================
- Coverage   39.17%   36.95%   -2.22%     
==========================================
  Files         299      268      -31     
  Lines        7025     6678     -347     
  Branches     1292     1203      -89     
==========================================
- Hits         2752     2468     -284     
+ Misses       3588     3557      -31     
+ Partials      685      653      -32
Impacted Files Coverage Δ
blocks/library/button/index.js 13.33% <100%> (-1.38%) ⬇️
blocks/block-controls/index.js 0% <0%> (-100%) ⬇️
components/panel/row.js 0% <0%> (-100%) ⬇️
editor/components/warning/index.js 0% <0%> (-100%) ⬇️
components/panel/color.js 0% <0%> (-100%) ⬇️
blocks/color-palette/index.js 0% <0%> (-90.91%) ⬇️
blocks/alignment-toolbar/index.js 16.66% <0%> (-83.34%) ⬇️
editor/components/post-last-revision/check.js 0% <0%> (-75%) ⬇️
editor/components/post-taxonomies/index.js 28.57% <0%> (-57.15%) ⬇️
editor/components/document-outline/index.js 17.85% <0%> (-57.15%) ⬇️
... and 217 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 976f8c6...b288588. Read the comment docs.

codecov bot commented Nov 12, 2017

Codecov Report

Merging #3445 into master will decrease coverage by 2.21%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3445      +/-   ##
==========================================
- Coverage   39.17%   36.95%   -2.22%     
==========================================
  Files         299      268      -31     
  Lines        7025     6678     -347     
  Branches     1292     1203      -89     
==========================================
- Hits         2752     2468     -284     
+ Misses       3588     3557      -31     
+ Partials      685      653      -32
Impacted Files Coverage Δ
blocks/library/button/index.js 13.33% <100%> (-1.38%) ⬇️
blocks/block-controls/index.js 0% <0%> (-100%) ⬇️
components/panel/row.js 0% <0%> (-100%) ⬇️
editor/components/warning/index.js 0% <0%> (-100%) ⬇️
components/panel/color.js 0% <0%> (-100%) ⬇️
blocks/color-palette/index.js 0% <0%> (-90.91%) ⬇️
blocks/alignment-toolbar/index.js 16.66% <0%> (-83.34%) ⬇️
editor/components/post-last-revision/check.js 0% <0%> (-75%) ⬇️
editor/components/post-taxonomies/index.js 28.57% <0%> (-57.15%) ⬇️
editor/components/document-outline/index.js 17.85% <0%> (-57.15%) ⬇️
... and 217 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 976f8c6...b288588. Read the comment docs.

@samikeijonen samikeijonen changed the title from update button block CSS and add class to link to Update button block CSS and add class to link Nov 13, 2017

@karmatosed

This comment has been minimized.

Show comment
Hide comment
@karmatosed

karmatosed Dec 20, 2017

Member

Can you refresh this PR so we can get it in for the button fix please?

Member

karmatosed commented Dec 20, 2017

Can you refresh this PR so we can get it in for the button fix please?

@@ -228,9 +230,16 @@ registerBlockType( 'core/button', {
save( { attributes } ) {

This comment has been minimized.

@youknowriad

youknowriad Dec 20, 2017

Contributor

Changing the save function means the old button blocks will become invalid, and we should declare a "deprecated version" to automatically upgrade these old blocks. (See the quote block as an example). Unfortunately, this API is not yet documented.

@youknowriad

youknowriad Dec 20, 2017

Contributor

Changing the save function means the old button blocks will become invalid, and we should declare a "deprecated version" to automatically upgrade these old blocks. (See the quote block as an example). Unfortunately, this API is not yet documented.

This comment has been minimized.

@samikeijonen

samikeijonen Dec 20, 2017

Contributor

I can try to check it after holidays.

@samikeijonen

samikeijonen Dec 20, 2017

Contributor

I can try to check it after holidays.

This comment has been minimized.

@samikeijonen

samikeijonen Jan 1, 2018

Contributor

In ad46294 I tried to declare a "deprecated version". I'm not sure how I could test this.

@samikeijonen

samikeijonen Jan 1, 2018

Contributor

In ad46294 I tried to declare a "deprecated version". I'm not sure how I could test this.

This comment has been minimized.

@youknowriad

youknowriad Jan 2, 2018

Contributor

You can test this by pasting in the code editor, the old markup. It should be converted to the new markup as soon as you focus out of the textarea.

@youknowriad

youknowriad Jan 2, 2018

Contributor

You can test this by pasting in the code editor, the old markup. It should be converted to the new markup as soon as you focus out of the textarea.

{ text }
</a>
</div>
);
},
deprecated: [ {

This comment has been minimized.

@youknowriad

youknowriad Jan 2, 2018

Contributor

It's not working for me, I believe you should assign the attributes definition to the deprecated version as well.

@youknowriad

youknowriad Jan 2, 2018

Contributor

It's not working for me, I believe you should assign the attributes definition to the deprecated version as well.

This comment has been minimized.

@samikeijonen

samikeijonen Jan 2, 2018

Contributor

I tried this in cdcafc3 but I think it's still not working.

@samikeijonen

samikeijonen Jan 2, 2018

Contributor

I tried this in cdcafc3 but I think it's still not working.

This comment has been minimized.

@youknowriad

youknowriad Jan 2, 2018

Contributor

It worked for me while testing.

@youknowriad

youknowriad Jan 2, 2018

Contributor

It worked for me while testing.

This comment has been minimized.

@samikeijonen

samikeijonen Jan 2, 2018

Contributor

Same here when tested again but sometimes it didn't for some reason:)

@samikeijonen

samikeijonen Jan 2, 2018

Contributor

Same here when tested again but sometimes it didn't for some reason:)

This comment has been minimized.

@youknowriad

youknowriad Jan 2, 2018

Contributor

I think there's probably a bug related to the deprecated blocks handling and the hooks (custom css class for example). But it's a separate issue.

@youknowriad

youknowriad Jan 2, 2018

Contributor

I think there's probably a bug related to the deprecated blocks handling and the hooks (custom css class for example). But it's a separate issue.

@youknowriad

LGTM 👍

@youknowriad youknowriad merged commit 365b932 into WordPress:master Jan 2, 2018

2 checks passed

codecov/project 39.18% (+0.01%) compared to 976f8c6
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

getsource added a commit to getsource/gutenberg that referenced this pull request Jan 3, 2018

@samikeijonen samikeijonen deleted the samikeijonen:update/button branch Jan 17, 2018

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