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

Fix widgets screen inserter #20680

Merged
merged 2 commits into from Mar 6, 2020
Merged

Fix widgets screen inserter #20680

merged 2 commits into from Mar 6, 2020

Conversation

@youknowriad
Copy link
Contributor

youknowriad commented Mar 6, 2020

This fixes the widgets screen inserter but also refactors the inserter style to avoid the global override of button styles.

Capture d’écran 2020-03-06 à 10 41 37 AM

@@ -51,21 +51,6 @@
line-height: $editor-line-height;
}

// Basic look for sibling inserter, and side-inserter.
.block-editor-inserter__toggle.components-button {

This comment has been minimized.

Copy link
@youknowriad

youknowriad Mar 6, 2020

Author Contributor

This was being applied to all inserters and not just the appender.
What we really want is to apply it to the inserters in the canvas.


&:hover {
color: $white;
}

This comment has been minimized.

Copy link
@youknowriad

youknowriad Mar 6, 2020

Author Contributor

This style is good but I wonder if this should be a button style variation (a prop in the Button component)

This comment has been minimized.

Copy link
@jasmussen

jasmussen Mar 6, 2020

Contributor

I think it should be. For example, we need a dark button style for a future Links iteration:

Screenshot 2020-03-06 at 11 32 19

Per this comment, we should also consider expanding the documentation to add more nuance than just "primary, secondary, tertiary" and so on. While there is certainly a visual hierarchy that an be used in the toolset of UI design, the markup is the same underneath, so it might make sense to look at variations than just an absolute hierarchy.

So perhaps a bigger undertaking than just adding a button style?

This comment has been minimized.

Copy link
@youknowriad

youknowriad Mar 6, 2020

Author Contributor

Not sure I understand, Technically hierarchy or variation seems like the same thing for me. It's just a matter of finding a name for it.

Design-wise, there needs to be some guidelines maybe (when to use what).

This comment has been minimized.

Copy link
@jasmussen

jasmussen Mar 6, 2020

Contributor

Right — but I mean you can have a primary black, primary blue, primary red, potentially. Just mean that we should not have a fourth level in the hierarchy.

This comment has been minimized.

Copy link
@youknowriad

youknowriad Mar 6, 2020

Author Contributor

Got it, you're right, that's more "theme support"

This comment has been minimized.

Copy link
@jasmussen

jasmussen Mar 6, 2020

Contributor

Yes but to be clear, I meant that even in the default style, the primary buttons are theme colored, AND the URL buttons are black, i.e. they coexist. And the red might exist as well, as a destructive action.

This comment has been minimized.

Copy link
@youknowriad

youknowriad Mar 6, 2020

Author Contributor

In that case, they seem more like variations to me.

primary, secondary, tertiary, error and the black one I can't name it.

This comment has been minimized.

Copy link
@youknowriad

youknowriad Mar 6, 2020

Author Contributor

or primary.hasError secondary.hasError...

This comment has been minimized.

Copy link
@jasmussen

jasmussen Mar 6, 2020

Contributor

Not something we have to decide right now, but it's something I know @mtias and @pablohoneyhoney have thought about.

@github-actions

This comment has been minimized.

Copy link

github-actions bot commented Mar 6, 2020

Size Change: -35 B (0%)

Total Size: 863 kB

Filename Size Change
build/block-editor/style-rtl.css 10.6 kB +35 B (0%)
build/block-editor/style.css 10.6 kB +36 B (0%)
build/edit-post/style-rtl.css 8.65 kB -55 B (0%)
build/edit-post/style.css 8.65 kB -58 B (0%)
build/edit-widgets/index.js 4.44 kB +20 B (0%)
build/edit-widgets/style-rtl.css 2.58 kB -7 B (0%)
build/edit-widgets/style.css 2.58 kB -6 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.01 kB 0 B
build/annotations/index.js 3.43 kB 0 B
build/api-fetch/index.js 3.39 kB 0 B
build/autop/index.js 2.58 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.02 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-editor/index.js 104 kB 0 B
build/block-library/editor-rtl.css 7.26 kB 0 B
build/block-library/editor.css 7.26 kB 0 B
build/block-library/index.js 115 kB 0 B
build/block-library/style-rtl.css 7.5 kB 0 B
build/block-library/style.css 7.51 kB 0 B
build/block-library/theme-rtl.css 669 B 0 B
build/block-library/theme.css 671 B 0 B
build/block-serialization-default-parser/index.js 1.65 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 57.7 kB 0 B
build/components/index.js 191 kB 0 B
build/components/style-rtl.css 15.5 kB 0 B
build/components/style.css 15.5 kB 0 B
build/compose/index.js 5.75 kB 0 B
build/core-data/index.js 10.6 kB 0 B
build/data-controls/index.js 1.03 kB 0 B
build/data/index.js 8.22 kB 0 B
build/date/index.js 5.36 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.06 kB 0 B
build/edit-post/index.js 91.3 kB 0 B
build/edit-site/index.js 4.64 kB 0 B
build/edit-site/style-rtl.css 2.48 kB 0 B
build/edit-site/style.css 2.48 kB 0 B
build/editor/editor-styles-rtl.css 325 B 0 B
build/editor/editor-styles.css 327 B 0 B
build/editor/index.js 43.8 kB 0 B
build/editor/style-rtl.css 3.98 kB 0 B
build/editor/style.css 3.98 kB 0 B
build/element/index.js 4.45 kB 0 B
build/escape-html/index.js 734 B 0 B
build/format-library/index.js 7.11 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 1.92 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.48 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.3 kB 0 B
build/keycodes/index.js 1.68 kB 0 B
build/list-reusable-blocks/index.js 2.99 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 4.85 kB 0 B
build/notices/index.js 1.57 kB 0 B
build/nux/index.js 3.01 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.54 kB 0 B
build/primitives/index.js 1.49 kB 0 B
build/priority-queue/index.js 780 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.3 kB 0 B
build/server-side-render/index.js 2.55 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4 kB 0 B
build/viewport/index.js 1.61 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.18 kB 0 B

compressed-size-action

Copy link
Contributor

jasmussen left a comment

Seems to work well for me, so almost good to go.

But now the inserter in the empty paragraph has the "delayed fade in" too:

Screenshot 2020-03-06 at 11 35 49

While I think that button on the right of the empty paragraph deserves a rethink, it seems worth keeping the behavior for now.

@youknowriad youknowriad merged commit e522fbc into master Mar 6, 2020
3 checks passed
3 checks passed
build
Details
pull-request-automation
Details
Travis CI - Pull Request Build Passed
Details
@youknowriad youknowriad deleted the fix/widgets-inserter branch Mar 6, 2020
@github-actions github-actions bot added this to the Gutenberg 7.7 milestone Mar 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.