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.

Sign up for GitHub

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

Jump to bottom

[AMP Stories] Add "Tip" block style for Text Block. #2068

Merged
merged 4 commits into from
Apr 4, 2019
Merged

[AMP Stories] Add "Tip" block style for Text Block. #2068

merged 4 commits into from
Apr 4, 2019

Conversation

miina
Copy link
Contributor

@miina miina commented Apr 4, 2019 •
edited by swissspidy
Loading

See #2013.

@googlebot googlebot added the cla: yes Signed the Google CLA label Apr 4, 2019
@miina miina requested a review from swissspidy April 4, 2019 16:24
swissspidy
swissspidy reviewed Apr 4, 2019
View reviewed changes
Copy link
Collaborator

@swissspidy swissspidy left a comment

Choose a reason for hiding this comment

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

Noticed: If the text block does not have a background color yet, the style previews are kinda pointless.

Thought: Should we rename "Tip" to "Half Rounded"? That's essentially what it is.

Result:

Screenshot 2019-04-04 at 18 17 52

@miina
Copy link
Contributor Author

miina commented Apr 4, 2019

Changed to "Half Rounded" :D
It is more descriptive, the word "Tip" without context is not clear.

@swissspidy swissspidy merged commit e35aa84 into amp-stories-redux Apr 4, 2019
@swissspidy swissspidy deleted the amp-story/text-block_tip-styling branch April 4, 2019 18:01
@westonruter
Copy link
Member

Should the CSS have a ::first-line rule that makes the first line a different style, like smaller font size?

@miina
Copy link
Contributor Author

miina commented Apr 4, 2019

Maybe it would be okay to use another Text Block in case of wanting to have different font size? This way the "Half Rounded" could be used more universally.

@westonruter westonruter added this to the v1.2 milestone May 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@swissspidy swissspidy swissspidy approved these changes

Assignees
No one assigned
Labels
cla: yes Signed the Google CLA
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.
4 participants
@miina @westonruter @swissspidy @googlebot