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

Fix issue with rich text toolbar being gone in captions #7050

Merged
merged 2 commits into from Jun 1, 2018

Conversation

Projects
None yet
3 participants
@jasmussen
Contributor

jasmussen commented May 31, 2018

Fixes #7049.

This changes how the caption field works in gallery items, so that the rich text toolbar is present. It was pushed upwards, and therefore hidden by an overflow rule. But that overflow rule was there to ensure that if you write an essay as a caption, the caption scrolls.

screen shot 2018-05-31 at 13 19 35

screen shot 2018-05-31 at 13 19 40

screen shot 2018-05-31 at 13 19 44

Fix issue with rich text toolbar being gone in captions
Fixes #7049.

This changes how the caption field works in gallery items, so that the rich text toolbar is present. It was pushed upwards, and therefore hidden by an overflow rule. But that overflow rule was there to ensure that if you write an essay as a caption, the caption scrolls.

@jasmussen jasmussen added the Design label May 31, 2018

@jasmussen jasmussen self-assigned this May 31, 2018

@jasmussen jasmussen requested review from gziolo and WordPress/gutenberg-core May 31, 2018

@tofumatt

Little nitpick requests, but otherwise looks good. 👍

@@ -34,6 +34,16 @@
width: calc( 100% - 8px );
left: 4px;
margin-top: -4px;
// if this inline toolbar has negative margin, it's hidden by the overflow that we need for scrolling long captions

This comment has been minimized.

@tofumatt

tofumatt May 31, 2018

Member

Serious nitpick but I think it's good for comments to be full sentences and start with capital letters.

This comment has been minimized.

@tofumatt

tofumatt May 31, 2018

Member

Also, this comment is a bit indirect in saying what the objective/reason for this CSS is. How about: "Override negative margins so this toolbar isn't hidden by overflow. Overflow is needed for long captions."

This comment has been minimized.

@jasmussen

jasmussen Jun 1, 2018

Contributor

I dig serious nitpicks! 👍 👍

// make extra space for the inline toolbar
figcaption {
padding-top: 48px;

This comment has been minimized.

@tofumatt

tofumatt May 31, 2018

Member

Does this have a class name we could target instead? Might be nice. If not no worries, I just always worry about targeting tag names for specificity… and possibly performance but that's super nitpick.

This comment has been minimized.

@jasmussen

jasmussen Jun 1, 2018

Contributor

It does, and I just pushed a thing to use it. I didn't do that previously because editor-rich-text__tinymce is used in a ton of places. But I noticed the Button uses that class to target a similar thing, so I simply used it properly scoped.

@jasmussen jasmussen added this to the 3.0 milestone Jun 1, 2018

@jasmussen

This comment has been minimized.

Contributor

jasmussen commented Jun 1, 2018

Thanks for the review! Addressed the feedback. Merging when the checks pass.

@jasmussen jasmussen merged commit 17f1963 into master Jun 1, 2018

2 checks passed

codecov/project 46.32% (-0.16%) compared to c93b778
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jasmussen jasmussen deleted the fix/gallery-descriptions branch Jun 1, 2018

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