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

Newly created paragraph block is not showing the font size. #7240

Closed

Conversation

andreyc0d3r
Copy link
Contributor

Newly added paragraph block is not showing the font size.

The font size is not set for components-range-control__number element. This commit fixes the issue.

…ze is not set for components-range-control__number element. This commit fixes the issue.
error  Unnecessary 'else' after 'return'  no-else-return
@andreyc0d3r andreyc0d3r closed this Jun 9, 2018
@andreyc0d3r andreyc0d3r reopened this Jun 9, 2018
@aduth
Copy link
Member

aduth commented Jun 9, 2018

As I understand it, this may have been an intentional choice to not have an explicit size assigned for the paragraph, but rather to inherit the size which would otherwise apply by default there, since 16px may not be the default depending upon the theme.

@aduth
Copy link
Member

aduth commented Jun 11, 2018

Related: #5269

@mtias , having introduced this, can you clarify whether it was intentional to not explicitly tie the default size to a fixed numeric value?

@aduth aduth added [Feature] Blocks Overall functionality of blocks Needs Decision Needs a decision to be actionable or relevant labels Jun 11, 2018
@gziolo gziolo requested a review from jorgefilipecosta June 18, 2018 12:35
@gziolo
Copy link
Member

gziolo commented Jun 18, 2018

I don't think this is expected to enforce the default size for all Paragraph blocks. As far as I remember from discussions with @jorgefilipecosta, it is possible to provide a different size using CSS, and we are able to detect it and set proper value, i.e. if a theme author changes the default font size to 17px, it will be set as a default value, but it won't force the block to append a class name or style which would duplicate the information. @jorgefilipecosta, can you confirm?

@gziolo gziolo requested a review from mtias June 18, 2018 12:39
@jorgefilipecosta
Copy link
Member

jorgefilipecosta commented Jun 18, 2018

The "number field" of size is blank to indicate to the user that no choice regarding the font-size was made.
By default not setting a font-size may render 16px, but if we change to another theme the default size may be other and paragraphs with the default size may change the size. Where if we change the font size to 16px explicitly if we change theme that value may not be the same as the default one.

We may improve this if we find a UI that allows the user to know the default font-size value e.g: 16 px but also shows that this value was not set it is just a default in that scenario and may change (something identical to a placeholder in a text field).

@aduth
Copy link
Member

aduth commented Jun 25, 2018

Based on previous comments, closing this pull request as invalid. Thanks for the contribution and for prompting the discussion!

@aduth aduth closed this Jun 25, 2018
@andreyc0d3r andreyc0d3r deleted the add/show-default-font-size branch July 10, 2018 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks Needs Decision Needs a decision to be actionable or relevant
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants