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

Blocks > Make AMP Stories block more granular by providing different blocks for different layout options #1530

Closed
6 tasks done
alcurrie opened this issue Oct 24, 2018 · 19 comments
Labels
Enhancement New feature or improvement of an existing one
Milestone

Comments

@alcurrie
Copy link

alcurrie commented Oct 24, 2018

As AMP story creator, I'd like the AMP Stories block to be more granular so I can split the grid layer, for different layout options.

  • AC 1: Split up the grid layer into separate blocks: vertical grid layer, thirds grid layer, horizontal grid layer, fill grid layer.
  • AC 2: The fill grid layer will be able to be further specialized with more preconfigured block configurations for background fill image layer and background fill video layer (NB: this will lay the foundation for user being able to save new block templates which are pre-populated with the common configurations of blocks that they use)
  • AC 3: Provide better defaults for blocks in AMP Stories, in particular the video block ie.:
    • Template is set to “fill”
    • AMP layout default value is set to “fill”
    • Autoplay (block option for autoplay/not auto-play
@alcurrie alcurrie self-assigned this Oct 24, 2018
@alcurrie
Copy link
Author

alcurrie commented Oct 24, 2018

Questions/Notes for design/interface discussion
AC 1 - Confirm interface change i.e. the current grid layer change from grid layer with templates (ie. thirds, horizontal) to grid layer types (ie. thirds grid layer,)
AC 2 - Confirm create 2 new preconfigured blocks' (ie. 1 with pre-configured block background fill image) and 1 with background fill video layer)
AC 3 - Example of 'better default for blocks' > video blocks. What other blocks need 'defaults' set and/or updated?

NB: this may need to be split out into separate issues.

@alcurrie
Copy link
Author

alcurrie commented Nov 1, 2018

@miina and @jwold - can you please take a look at the AC in this issue as well as the Questions/Notes for discussion. I think this issue may need more elaboration/specifics.

cc. @mehigh

@alcurrie alcurrie removed their assignment Nov 1, 2018
@jwold
Copy link
Collaborator

jwold commented Nov 1, 2018

Our understanding is that this issue is for the UX to only create icons for each of the new blocks,
Therefore we'll just focus on creating the icons at this time.

This is not blocked by UX for dev to begin.

@jwold
Copy link
Collaborator

jwold commented Nov 5, 2018

Icons needed:

  • Image fill layer
  • Video fill layer
  • Thirds layer
  • Vertical layer
  • Horizontal layer
  • CTA Grid layer

@miina
Copy link
Contributor

miina commented Nov 7, 2018

@jwold Assigning this to you as well since there are icons needed, the dev part should be ready (everything except for implementing the icons).

@jwold
Copy link
Collaborator

jwold commented Nov 8, 2018

Also need to create separate icons for fill image and fill video.

@jwold
Copy link
Collaborator

jwold commented Nov 8, 2018

screen shot 2018-11-08 at 10 47 55 am

Would love some feedback on this! :) @miina, @alcurrie, @cathibosco

@jwold
Copy link
Collaborator

jwold commented Nov 8, 2018

Todo for later: we may want to make these stand out a bit differently in the inserter. For instance (crazy idea) adding a blue background to all items in the inserter that are for AMP Stories.

@jwold jwold removed the UX label Nov 8, 2018
@jwold jwold removed their assignment Nov 8, 2018
@jwold
Copy link
Collaborator

jwold commented Nov 8, 2018

Going to remove UX feedback for now, but happy to jump back in if need be!

@miina
Copy link
Contributor

miina commented Nov 8, 2018

@jwold Thanks for the icons and for the thoughts, separating AMP Story blocks would be great, especially until it's not possible to hide all the non-AMP Story blocks.

It's possible to create a separate category for AMP Story blocks as well, so they would all appear separately grouped :)

One note on the icons: would it be possible to get these as .svg files, too? Whom should I ask for that?

Thank you!

@jwold
Copy link
Collaborator

jwold commented Nov 8, 2018

  1. @miina I love the idea of putting them in a separate category. That should be enough.
  2. Here are the SVGs, if you need any changes, or resizes, etc, let me know! :)

Download link: svg-icons.zip

@jwold
Copy link
Collaborator

jwold commented Nov 8, 2018

Note: we ran into an issue with the SVG importing. @cathibosco is helping with making the SVG updates, which we'll post back to this issue.

@cathibosco
Copy link

Revised SVG icon package version2
Archive.zip

@miina
Copy link
Contributor

miina commented Nov 9, 2018

Fixed within #1594.

@miina miina closed this as completed Nov 9, 2018
@miina miina assigned alcurrie and unassigned miina Nov 14, 2018
@alcurrie
Copy link
Author

alcurrie commented Dec 2, 2018

Confirmed AC 1 and 2: splitting out into separate layer blocks, renaming layer blocks and updating the icons: https://cl.ly/0702f12301c6
I've confirmed that adding a YouTube video 'fills' the width of the page:
I've confirmed the improved defaults: https://cl.ly/7c8a49cb873c

2 questions:

  1. @miina I don't see an autoplay option in the video player block options is there a reason this could not be implemented, or was this just missed?
  2. @jwold / @miina to get the fill to 'fill the screen for AMP stories, is there a recommended size, and if so, is it possible to add that note to the interface. If not, let's make sure to add that to the wiki

@alcurrie alcurrie assigned miina and alcurrie and unassigned alcurrie Dec 2, 2018
@miina
Copy link
Contributor

miina commented Dec 3, 2018

@alcurrie:

  1. Note that for the Video Settings to appear a video needs to be added first. For example if you add a Fill Video Layer and then add a video, this appears with the Autoplay option:
    screen shot 2018-12-03 at 11 15 38 am
  2. Recommended size is 720x1280 px -- probably would be good to add this to Wiki for now and leave messages etc. for the next phase, thoughts?

@alcurrie
Copy link
Author

alcurrie commented Dec 3, 2018

Thanks @miina - I'll retest and add a note for the Wiki. Re. messages for the 'next phase' do you mean that we should consider adding support for other videos sizes for the next phase, or are there other video related enhancements that we should be tracking?

@miina
Copy link
Contributor

miina commented Dec 3, 2018

@alcurrie I'm thinking that probably generally in the next phase we could look over all the existing features and see if we could make things easier for the editor by displaying relevant messages for best practices and recommendations for AMP Stories. Such as display the recommended image size / ratio and the same for video, etc. Thoughts?

@alcurrie
Copy link
Author

alcurrie commented Dec 5, 2018

That's a great idea @miina. Let's track that that for next phase I've made a note to follow up after WCUS. I'll update the notes for the WIKI as well

However this issue is confirmed complete, with the confirmation of the Video Autoplay. : https://cl.ly/3b53097e0789

@westonruter westonruter added this to the v1.2 milestone May 21, 2019
@swissspidy swissspidy added Enhancement New feature or improvement of an existing one and removed AMP-Stories-Extension labels Jun 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or improvement of an existing one
Projects
None yet
Development

No branches or pull requests

6 participants