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

[AMP Stories] Make sure that the frontend and editor view match #1967

Closed
6 tasks done
miina opened this issue Mar 14, 2019 · 7 comments
Closed
6 tasks done

[AMP Stories] Make sure that the frontend and editor view match #1967

miina opened this issue Mar 14, 2019 · 7 comments
Projects
Milestone

Comments

@miina
Copy link
Contributor

miina commented Mar 14, 2019

AC: All the allowed blocks in the editor look visually the same (or very similar) to how they look like in the frontend. This does not include animations.

  • Make sure that all the text containing blocks' font sizes in the editor match the sizes in frontend;
  • Make sure that the margins and paddings in the editor match the frontend. For example, the AMP Story Grid Layer seems to have padding but this is not reflected in the editor;
  • Make sure that the Text block displays similarly in the editor as in the frontend. At this moment a Text block might display 2 rows in the editor but just one in the frontend.
  • Verify other blocks not mentioned here separately.
  • Make sure the ratio of the Page is correct (currently per Weston's comment Frontend story is: 390.141/650.250 = 0.59998615917, however, Editor is: 338/553 = 0.61121157323)
  • Add basic editor style (e.g. max-width).
@westonruter
Copy link
Member

See also discussion about how we should include styles on the frontend: #1958 (comment)

@swissspidy
Copy link
Collaborator

Make sure the ratio of the Page is correct

Note that this also affects the reordering UI.

@MackenzieHartung MackenzieHartung moved this from Backlog to To Do in AMP Stories Mar 18, 2019
@miina miina moved this from To Do to In progress in AMP Stories Mar 20, 2019
@miina miina moved this from In progress to To Do in AMP Stories Mar 20, 2019
@miina miina self-assigned this Mar 21, 2019
@miina miina moved this from To Do to In progress in AMP Stories Mar 21, 2019
@miina
Copy link
Contributor Author

miina commented Mar 21, 2019

I'm wondering what would be the best approach for font sizes -- for example if the default font size is 16px in the editor and is displayed in two rows then the same font size in the frontend might still be in one row -- since the font size is not responsive. We could add some different sizes based on the screen width for the default font sizes, however, what if the user selects a specific font size?

@swissspidy @westonruter Thoughts?

@miina miina moved this from In progress to Ready for Review in AMP Stories Mar 22, 2019
@westonruter
Copy link
Member

@miina What if the specific font sizes were stored in vh units instead of px units? That would make them responsive. They'd perhaps have to remain px in the editor since the the viewport is not fully contained by the story, but when rendered on the frontend they could be replaced with some computed equivalent using vh. This would make the font size scale based on the height of the viewport, and thus it would be responsive.

@miina
Copy link
Contributor Author

miina commented Mar 25, 2019

@westonruter: Just updated the PR to use vw units instead of px for the font size in the frontend, the editor still displays in px.

@miina
Copy link
Contributor Author

miina commented Mar 26, 2019

Closed within #2015.

@miina miina closed this as completed Mar 26, 2019
@miina miina moved this from Ready for Review to Ready for Merging in AMP Stories Mar 26, 2019
@miina miina moved this from Ready for Merging to Ready for QA in AMP Stories Apr 10, 2019
@miina miina removed their assignment Apr 10, 2019
@csossi
Copy link

csossi commented Apr 15, 2019

verified in QA

@csossi csossi moved this from Ready for QA to Ready for Merging in AMP Stories Apr 15, 2019
@westonruter westonruter added this to the v1.2 milestone May 21, 2019
@ThierryA ThierryA moved this from Ready for Merging to Done in AMP Stories Jun 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

5 participants