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 > Do not allow more than one block inside the AMP Grid layer when the 'fill' template is selected #1526

Closed
1 of 4 tasks
alcurrie opened this issue Oct 23, 2018 · 11 comments
Labels
Enhancement New feature or improvement of an existing one UX
Milestone

Comments

@alcurrie
Copy link

alcurrie commented Oct 23, 2018

As a editor/author creating an AMP story I want to be prevented from adding more than one block to the AMP Grid Layer when using the 'fill template' so that I don't hide all the 'other' blocks by mistake.

  • AC 1: When the fill template is selected for the AMP Grid Layer, only 1 block can be added to the Grid Layer
  • AC2: Appropriate messaging in the interface will let the user know that when using the 'fill template' on the AMP Grid Layer, only 1 block can be added to the AMP Grid layer
  • AC3: If the user tries to add a 2nd block to a AMP Grid layer with the template set to fill, a warning will display for the user and the user will not able able to add the 2nd block
  • AC4: For other templates (i.e. the vertical one) multiple blocks can be added to an AMP Grid layer, and blocks can be laid out in relation to each other

Background on this change
This brings the functionality in alignment with the expected functionality on the FE and avoids user confusion when the template fill seems to ‘hide’ all other blocks except the first one. From a user perspective the AMP Grid Layer with Template ‘Fill’ hides all the other blocks except the first one. This can cause issues using the default paragraph block in a layer when background image fill is set. ie. see this example where the image block added first covers/hides the 'text' block. https://d.pr/i/gHSAxO

@jwold
Copy link
Collaborator

jwold commented Nov 1, 2018

This one will need some design feedback in terms of the messaging. We'll need to look into what kind of messaging option Gutenberg has now and then verify if we should change it.

@miina
Copy link
Contributor

miina commented Nov 1, 2018

Note for dev: Development can be started independent of UX/Design work an consulted later to see if the existing messaging options are suitable as they are.

Note that this ticket is blocked by #1530 since after creating different blocks then AMP Layer fill will be a type of block instead of being able to select a fill template.

@miina miina added the BE label Nov 1, 2018
@jwold
Copy link
Collaborator

jwold commented Nov 2, 2018

Had a chat with @alcurrie today and wanted to share a sketch that walks through the discussion. Currently we want to make sure that we're actively stopping users from doing the following:

  1. If I have a block inside a fill layer (presumably an image?), I should not be able to add a second block by clicking the plus button in the header of the page.
  2. If I have a block inside a fill layer, such as a paragraph, should not be able to press return/enter below the image to start a new paragraph or type "/" to bring up the block inserter interface. A warning should probably popup when the user tries this.
  3. We still need to figure out the appropriate message that pops up, and what it looks like. For instance, if I select a fill layer, maybe a message that shows originally saying only one block can be here, then a warning shows up when I try to go against that.

my sketches - 2018-11-02 07 59 06

@miina
Copy link
Contributor

miina commented Nov 2, 2018

@jwold Thanks for sharing the sketches and detailed thoughts for different cases.
On 1: Yes, most likely it would have either a video or an image.

Some thoughts:
For the 1.5 maybe we could think of as simple way as possible with the goal of providing a bug-less experience and not confusing the user but at the same time considering that "an ideal" experience could be implemented in Phase 2 and wouldn't necessarily have to be part of 1.5, unless it's simple to implement.

For example one option could be that in case of Fill Layer we would always display a static message saying something like: "Note that Fill Layer can have only one inner element. Any additional blocks will be removed automatically."

This would mean that we wouldn't have to worry about different cases for pressing return/enter of typing in / -- even if they'd do that we'd just remove the block right after adding it.

Do you think this might still be confusing? Thoughts?

cc @alcurrie

@jwold jwold assigned jwold and alcurrie and unassigned jwold Nov 5, 2018
@jwold
Copy link
Collaborator

jwold commented Nov 5, 2018

If a user tries to add a second image into an image fill layer (this also applies for video fill layer), then the object will be automatically removed, and an error message will appear below the header, indicating that this isn't allowed.

Example of what the error could look like:

screen shot 2018-11-05 at 11 50 47

@miina miina self-assigned this Nov 6, 2018
@miina
Copy link
Contributor

miina commented Nov 6, 2018

@alcurrie @jwold Note that this issue will likely get resolved within #1530 and might not need messaging afterall due to being able to just disable the Inserter.

I'll let you know in case it changes but at this moment UX changes shouldn't be needed at all. I'll move this ticket to "In Progress".

@miina
Copy link
Contributor

miina commented Nov 9, 2018

Fixed within #1594.

@miina miina closed this as completed Nov 9, 2018
@miina miina removed their assignment Nov 14, 2018
@alcurrie
Copy link
Author

alcurrie commented Dec 3, 2018

The user is not blocked from adding a new block to an image fill layer using the 'insert block'. See screencast: https://cl.ly/24e2f604ef79

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

miina commented Dec 3, 2018

@alcurrie The screencast shows that the paragraph is created after the Image Fill Layer (inside Page block) but not inside the image Fill Layer.

The AC1 here is for not being able to add more than one block into Fill Layer which seems to be true -- it's not possible to add another block inside the Image Fill Layer.

The bug that you found seems to be a separate general issue, not a Fill Layer related issue, and would probably need a new ticket.

The issue title could be something like: It should not be possible to add any other blocks than Layer blocks and CTA blocks inside the Page block.

@miina miina assigned alcurrie and unassigned miina Dec 3, 2018
@alcurrie
Copy link
Author

alcurrie commented Dec 4, 2018

Thanks for clarifying @miina -I'll move this back into QA and re-test, and check the 'other' issue and add a new ticket.

@alcurrie
Copy link
Author

alcurrie commented Dec 5, 2018

I've confirmed the AC for adding an image fill layer are met, as follows:
AC 1: When the fill image template is selected - adding an image interface displays by default and the user can add/update only 1 image
AC2: and AC 3: Instead of 'messaging' the user is only given the single option of uploading a image in the Fill image layer and there are no other options to add other blocks. So the user will not able able to add a 2nd block to the image fill layer
https://cl.ly/bf66b8ee722b
AC4: For other templates (i.e. the vertical one) multiple blocks can be added to an AMP Grid layer, and blocks can be laid out in relation to each other - confirmed

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 UX
Projects
None yet
Development

No branches or pull requests

5 participants