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 plus. #22704

Merged
merged 1 commit into from
Jun 16, 2020
Merged

Fix plus. #22704

merged 1 commit into from
Jun 16, 2020

Conversation

jasmussen
Copy link
Contributor

The plus in Buttons, Social Links, and Navigation regressed:

Screenshot 2020-05-28 at 13 21 06

Screenshot 2020-05-28 at 12 47 06

This fixes that:

Screenshot 2020-05-28 at 13 29 53

However there's one niggle. We use the same component for the appender in buttons/sociallinks/navigation, as we do for the big generic appender in columns. The latter we intentionally changed to use the smaller plus button, the icon called create. This PR actually regesses that, so both of those use the same icon:

Screenshot 2020-05-28 at 13 28 38

I couldn't immediately figure out how to serve one or the other depending on whether it was the big white background version, or the small black version. Input appreciated.

@jasmussen jasmussen added the [Type] Bug An existing feature does not function as intended label May 28, 2020
@jasmussen jasmussen self-assigned this May 28, 2020
@github-actions
Copy link

github-actions bot commented May 28, 2020

Size Change: -39 B (0%)

Total Size: 1.13 MB

Filename Size Change
build/block-editor/index.js 106 kB -43 B (0%)
build/block-editor/style-rtl.css 12.1 kB +2 B (0%)
build/block-editor/style.css 12.1 kB +2 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 3.4 kB 0 B
build/autop/index.js 2.83 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.27 kB 0 B
build/block-directory/style-rtl.css 892 B 0 B
build/block-directory/style.css 892 B 0 B
build/block-library/editor-rtl.css 7.88 kB 0 B
build/block-library/editor.css 7.89 kB 0 B
build/block-library/index.js 129 kB 0 B
build/block-library/style-rtl.css 7.96 kB 0 B
build/block-library/style.css 7.96 kB 0 B
build/block-library/theme-rtl.css 684 B 0 B
build/block-library/theme.css 686 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 195 kB 0 B
build/components/style-rtl.css 19.5 kB 0 B
build/components/style.css 19.5 kB 0 B
build/compose/index.js 9.32 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.44 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.17 kB 0 B
build/edit-navigation/index.js 8.26 kB 0 B
build/edit-navigation/style-rtl.css 974 B 0 B
build/edit-navigation/style.css 974 B 0 B
build/edit-post/index.js 302 kB 0 B
build/edit-post/style-rtl.css 5.6 kB 0 B
build/edit-post/style.css 5.6 kB 0 B
build/edit-site/index.js 16.6 kB 0 B
build/edit-site/style-rtl.css 2.96 kB 0 B
build/edit-site/style.css 2.96 kB 0 B
build/edit-widgets/index.js 9.34 kB 0 B
build/edit-widgets/style-rtl.css 2.4 kB 0 B
build/edit-widgets/style.css 2.4 kB 0 B
build/editor/editor-styles-rtl.css 423 B 0 B
build/editor/editor-styles.css 423 B 0 B
build/editor/index.js 44.8 kB 0 B
build/editor/style-rtl.css 4.26 kB 0 B
build/editor/style.css 4.27 kB 0 B
build/element/index.js 4.64 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.72 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 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.13 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 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 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.56 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14 kB 0 B
build/server-side-render/index.js 2.68 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@jasmussen jasmussen requested a review from a team June 12, 2020 06:17
@youknowriad
Copy link
Contributor

Pinging @adamziel just to ensure that we're not going back on forth here.

@adamziel
Copy link
Contributor

adamziel commented Jun 12, 2020

I'm not sure, looping in @shaunandrews, @draganescu, and @tellthemachines

Copy link
Contributor

@draganescu draganescu left a comment

Choose a reason for hiding this comment

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

As far as I've seen the create icon has been introduced in #21476 so it is not a feature connected to Navigation or Buttons PRs. I can reproduce this on master:

Screenshot 2020-06-15 at 14 40 16

and this PR fixes that.

@shaunandrews
Copy link
Contributor

Looks good to me.

This PR actually regresses that, so both of those use the same icon:

I'm not familiar with the decision behind the two different icons, but the consistency here seems good.

@jasmussen
Copy link
Contributor Author

Thanks for the reviews and sanity checks. I'll rebase and merge when I have a moment.

@jasmussen jasmussen merged commit 3f65d98 into master Jun 16, 2020
@jasmussen jasmussen deleted the fix/plus branch June 16, 2020 08:00
@github-actions github-actions bot added this to the Gutenberg 8.4 milestone Jun 16, 2020
@mtias
Copy link
Member

mtias commented Jun 17, 2020

Thanks :)

This was referenced Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants