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

Fix regression with textbox spacing + a focus issue #9186

Merged
merged 2 commits into from
Aug 22, 2018
Merged

Fix regression with textbox spacing + a focus issue #9186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5ec8554
Fix regression with textbox spacing + a focus issue
Aug 21, 2018
2c5d6af
Center text on copy buttons.
Aug 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions edit-post/assets/stylesheets/main.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,6 @@ body.gutenberg-editor-page {
input[type="number"],
select,
textarea {
margin-top: 0; // These override a "margin: 1px" from core.
margin-bottom: 0;
font-family: $default-font;
font-size: $default-font-size;
padding: 6px 8px;
Expand Down
13 changes: 0 additions & 13 deletions edit-post/components/visual-editor/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,19 +115,6 @@
}
}

// This is a focus style shown for blocks that need an indicator even when in an isEditing state
// like for example an image block that receives arrowkey focus.
.edit-post-visual-editor .editor-block-list__block:not(.is-selected) {
Copy link
Member

Choose a reason for hiding this comment

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

Is this okay to remove for a11y? Why did they need that state?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a bit hard to explain, there are GIFs in the archives here. But here's a GIF of the behavior today:

focus

Back in the day, the "isEditing" mode that fades out UI as you're typing remained faded out even when you navigated into a non text block. This would then get a black outline as I just commented out.

Copy link
Member

Choose a reason for hiding this comment

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

Coolio, that makes sense. Good to 🚢 in my mind!

Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we losing the "isEditing" mode when we move to a non-textual block at the moment?

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 can't recall the details but I believe it was an accessibility thing about the toolbar. I think maybe @aduth worked on it but not sure.

Copy link
Member

Choose a reason for hiding this comment

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

Context: #5552

Notably, as otherwise:

" it would be impossible to use the image toolbar by keyboard alone "

Copy link
Member

Choose a reason for hiding this comment

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

The function comment for ObserveTyping's stopTypingOnNonTextField could probably be improved to make clearer its purpose.

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 would also note that it could be revisited in the future, perhpas with some of the focus mode toolbar improvements. But I would also admit this to be low priority.

.editor-block-list__block-edit {
box-shadow: 0 0 0 0 $white, 0 0 0 0 $dark-gray-900;
transition: 0.1s box-shadow 0.05s;
}

&:focus .editor-block-list__block-edit {
box-shadow: 0 0 0 1px $white, 0 0 0 3px $dark-gray-900;
}
}

.edit-post-visual-editor .editor-default-block-appender {
// Default to centered and content-width, like blocks
max-width: $content-width;
Expand Down
2 changes: 1 addition & 1 deletion packages/editor/src/components/post-publish-panel/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
}

.components-button {
text-align: center;
justify-content: center;
Copy link
Member

Choose a reason for hiding this comment

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

Odd, justify-content says it doesn't work in IE 11 at all and is buggy in Edge with Flexbox. https://caniuse.com/#search=justify-content

But it looks good to me in IE11:

screenshot 2018-08-21 09 21 13

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Text align did nothing, for whatever reason.

}

.components-clipboard-button {
Expand Down