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

Post Comments Counts: Add style attributes #24167

Merged
merged 2 commits into from
Jul 27, 2020

Conversation

Copons
Copy link
Contributor

@Copons Copons commented Jul 23, 2020

Description

Update the Post Comments Counts block:

  • Add support for colors, font size, line height, and text alignment.
  • Convert the to light block wrapper.
  • Output a div instead of a span.

Note: the block simply outputs the number of comments, without labels.

How has this been tested?

Tested in Post, Page, and Site Editor, on Chrome 83 on macOS 10.15.5.
Tested on posts and pages, with comments allowed and otherwise.

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@github-actions
Copy link

github-actions bot commented Jul 23, 2020

Size Change: +147 B (0%)

Total Size: 1.15 MB

Filename Size Change
build/block-directory/index.min.js 7.63 kB -2 B (0%)
build/block-editor/index.min.js 125 kB +5 B (0%)
build/block-library/index.min.js 132 kB +47 B (0%)
build/blocks/index.min.js 48.2 kB -3 B (0%)
build/components/index.min.js 200 kB +3 B (0%)
build/data/index.min.js 8.45 kB -2 B (0%)
build/edit-post/index.min.js 304 kB +20 B (0%)
build/edit-site/index.min.js 17 kB +37 B (0%)
build/edit-widgets/index.min.js 9.38 kB +8 B (0%)
build/editor/index.min.js 45.3 kB +23 B (0%)
build/keyboard-shortcuts/index.min.js 2.52 kB +7 B (0%)
build/list-reusable-blocks/index.min.js 3.12 kB -2 B (0%)
build/nux/index.min.js 3.4 kB -5 B (0%)
build/plugins/index.min.js 2.56 kB -1 B
build/primitives/index.min.js 1.41 kB +4 B (0%)
build/rich-text/index.min.js 13.9 kB +8 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.min.js 1.14 kB 0 B
build/annotations/index.min.js 3.67 kB 0 B
build/api-fetch/index.min.js 3.43 kB 0 B
build/autop/index.min.js 2.82 kB 0 B
build/blob/index.min.js 620 B 0 B
build/block-directory/style-rtl.css 953 B 0 B
build/block-directory/style.css 952 B 0 B
build/block-editor/style-rtl.css 10.8 kB 0 B
build/block-editor/style.css 10.8 kB 0 B
build/block-library/editor-rtl.css 7.63 kB 0 B
build/block-library/editor.css 7.63 kB 0 B
build/block-library/style-rtl.css 7.83 kB 0 B
build/block-library/style.css 7.83 kB 0 B
build/block-library/theme-rtl.css 728 B 0 B
build/block-library/theme.css 729 B 0 B
build/block-serialization-default-parser/index.min.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.min.js 3.1 kB 0 B
build/components/style-rtl.css 15.7 kB 0 B
build/components/style.css 15.6 kB 0 B
build/compose/index.min.js 9.68 kB 0 B
build/core-data/index.min.js 11.5 kB 0 B
build/data-controls/index.min.js 1.29 kB 0 B
build/date/index.min.js 5.38 kB 0 B
build/deprecated/index.min.js 772 B 0 B
build/dom-ready/index.min.js 568 B 0 B
build/dom/index.min.js 3.23 kB 0 B
build/edit-navigation/index.min.js 10.8 kB 0 B
build/edit-navigation/style-rtl.css 1.08 kB 0 B
build/edit-navigation/style.css 1.08 kB 0 B
build/edit-post/style-rtl.css 5.61 kB 0 B
build/edit-post/style.css 5.61 kB 0 B
build/edit-site/style-rtl.css 3.06 kB 0 B
build/edit-site/style.css 3.06 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/style-rtl.css 3.78 kB 0 B
build/editor/style.css 3.78 kB 0 B
build/element/index.min.js 4.65 kB 0 B
build/escape-html/index.min.js 733 B 0 B
build/format-library/index.min.js 7.72 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.min.js 2.13 kB 0 B
build/html-entities/index.min.js 621 B 0 B
build/i18n/index.min.js 3.56 kB 0 B
build/is-shallow-equal/index.min.js 711 B 0 B
build/keycodes/index.min.js 1.94 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.min.js 5.33 kB 0 B
build/notices/index.min.js 1.79 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/priority-queue/index.min.js 789 B 0 B
build/redux-routine/index.min.js 2.85 kB 0 B
build/server-side-render/index.min.js 2.71 kB 0 B
build/shortcode/index.min.js 1.7 kB 0 B
build/token-list/index.min.js 1.27 kB 0 B
build/url/index.min.js 4.06 kB 0 B
build/viewport/index.min.js 1.85 kB 0 B
build/warning/index.min.js 1.14 kB 0 B
build/wordcount/index.min.js 1.17 kB 0 B

compressed-size-action

Copy link
Contributor

@Addison-Stavlo Addison-Stavlo left a comment

Choose a reason for hiding this comment

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

This looks good and works well for me. All concerns are regarding designs that will need to be addressed in the future.

Initially this was a span on the front end, I don't know if this made sense or not. Since we are adding feature parity in comparison with the paragraph block, we probably do want it as a block element. It seems a bit empty as a block element, but perhaps in future iterations we can add a customizable text that can be added before or after the number, so someone could customize the block more like:
This many comments: 0
or
This post has 0 comments!
😆

@Copons
Copy link
Contributor Author

Copons commented Jul 27, 2020

This looks good and works well for me. All concerns are regarding designs that will need to be addressed in the future.

Initially this was a span on the front end, I don't know if this made sense or not. Since we are adding feature parity in comparison with the paragraph block, we probably do want it as a block element. It seems a bit empty as a block element, but perhaps in future iterations we can add a customizable text that can be added before or after the number, so someone could customize the block more like:
This many comments: 0
or
This post has 0 comments!
😆

Yeah, having a custom label (maybe even with a custom %d token inside!) is definitely needed.
I briefly questioned my own decision of swapping from span to div, but in the end, we are living in the 2020, and CSS nowadays is capable enough to turn a div into an inline element. 😄

@Copons Copons merged commit 42554d1 into master Jul 27, 2020
@Copons Copons deleted the update/post-comments-count-style branch July 27, 2020 10:31
@github-actions github-actions bot added this to the Gutenberg 8.7 milestone Jul 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Post Comments Count Affects the Post Comments Count Block
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants