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

Fixed image add link input disappears issue #5942

Merged
merged 4 commits into from
Apr 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions blocks/library/image/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,14 @@
}
}
}

.editor-block-list__block[data-type="core/image"] .editor-block-toolbar .blocks-format-toolbar__link-modal {
position: absolute;
left: 0;
right: 0;
margin: -1px 0;

@include break-small() {
margin: -1px;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Files should end in newline.

Tip: Enforced automatically if installing an EditorConfig plugin.

9 changes: 9 additions & 0 deletions blocks/url-input/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ $input-size: 230px;
width: $input-size + $icon-button-size * 2 + 2px; // add borders
}

// Hide suggestions on mobile until we @todo find a better way to show them
.blocks-url-input__suggestions,
.blocks-url-input .spinner {
display: none;
@include break-small() {
display: inherit;
}
}

.blocks-url-input__suggestion {
padding: 4px #{ $icon-button-size + $input-padding } 4px $input-padding;
color: $dark-gray-300; // lightest we can use for contrast
Expand Down
4 changes: 0 additions & 4 deletions editor/components/block-list/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -670,10 +670,6 @@
margin-left: -1px;
}

> .editor-block-switcher:first-child {
margin-left: -2px;
}

@include break-small() {
width: auto;
}
Expand Down
4 changes: 4 additions & 0 deletions editor/components/block-switcher/style.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
.editor-block-switcher {
position: relative;

.components-toolbar {
border-left: none;
}
}

.editor-block-switcher__toggle {
Expand Down
7 changes: 6 additions & 1 deletion editor/components/block-toolbar/style.scss
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
.editor-block-toolbar {
display: inline-flex;
overflow: auto; // allow horizontal scrolling on mobile
flex-grow: 1;
width: 100%;
background: $white;
overflow: auto; // allow horizontal scrolling on mobile

// allow overflow on desktop
@include break-small() {
overflow: inherit;
}

.components-toolbar {
border: none;
Expand Down
3 changes: 2 additions & 1 deletion editor/components/observe-typing/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ class ObserveTyping extends Component {

// Abort early if already typing, or key press is incurred outside a
// text field (e.g. arrow-ing through toolbar buttons).
if ( isTyping || ! isTextField( target ) ) {
// Ignore typing in a block toolbar
if ( isTyping || ! isTextField( target ) || target.closest( '.editor-block-toolbar' ) ) {
Copy link
Member

@aduth aduth Apr 18, 2018

Choose a reason for hiding this comment

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

I understand this was merged because it's a bug, but we should be more explicit about when typing should and should not be triggered. It may well be that this is the correct approach to look for a toolbar ancestor, though could also be that it's a single fix to a larger problem. Are there other text fields in editor which shouldn't trigger isTyping ?

One such example on a quick glance is the inserter search for the default block, where the inserter button inexplicably disappears upon typing:

search

I originally liked that ObserveTyping was moved out of the block component because it's something of a global behavior, and BlockListBlock already has an abundant number of concerns, but am thinking more and more that it should at least be scoped in some way to the contents of a block. This still leaves open some possibility for issues with things like link inputs in RichText though.

Copy link
Contributor

Choose a reason for hiding this comment

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

This still leaves open some possibility for issues with things like link inputs in RichText though.

I think triggering isTyping in that case is the correct behavior.

--
For me, All inputs and richtext in block edit functions should trigger the behavior. Side UI shouldn't. All side UI is triggered is shown outside the editor content area (Popovers) aside the toolbar, so the fix made sense.

But I understand that wrapping only the BlockEdit component with this might also fix the issue. It would have been a good approach too.

return;
}

Expand Down