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

Cover block: Add media settings(fixed background and focal point picker) #1741

Closed
pinarol opened this issue Jan 6, 2020 · 19 comments · Fixed by #3028, wordpress-mobile/WordPress-iOS#15665 or wordpress-mobile/WordPress-Android#13818
Assignees
Labels

Comments

@pinarol
Copy link
Contributor

pinarol commented Jan 6, 2020

Depends on

Add Cover Block - 1st iteration

Mobile equivalent of Media settings needs to be added to Cover block:

Screen Shot 2020-01-06 at 14 22 30

cc @iamthomasbishop

@pinarol pinarol added [Type] Enhancement Improves a current area of the editor [Status] Needs Design labels Jan 6, 2020
@iamthomasbishop
Copy link
Contributor

@pinarol As discussed in Slack, I spent some time the other day sketching out how we might handle focal point, fixed background, and the wider lot of Cover block settings. I’ll zoom out a little bit to capture thoughts about how these things fit into the flow, while still trying to keep focused on the primary elements — fixed background and focal point editing.

Fixed background

image

From a design perspective, this one is pretty straightforward because we can re-use the native Switch component in a cell on the settings bottom sheet. In terms of placement, I personally think we might want to reconsider where we place its table cell, but more on that later.

Focal point

image

The UI on the web for editing the focal point is pretty intuitive. The drag handle for position adjustments also is a nice parallel to our UI for custom color picking (on the Button block, for example), where you have a relatively large surface on which you can move a small circular drag handle. That same concept should apply here, except the surface is a static image instead of a color palette.

The web also has text inputs (% values) for “horizontal position” and “vertical position”, which we could support with our Slider component.

Grouping it all into a Media section

image

On a high level, what this does is adds a new Media section and groups all of the media-centric settings, naturally titled “Media”. Contained within this section:

  • Media Item: A static version of the applied media item. If no media is applied, we would show a single button labeled “Add image or video”. If an image or video is applied, it would have an edit button over the top right of it, in the same way that we show this button on Image blocks and soon Cover blocks.
  • Edit Focal Point (button w/ disclosure): Below the media preview would be a row titled “Edit focal point”. When tapped, this transitions to a sub-sheet in the same way our color parent transitions to the custom color picker.
  • Fixed Background (switch:) Below the “edit focal point” button would be a Switch to toggle the “Fixed background” setting. This would have the same affect as is seen on the web.
  • Remove/clear Media (button): The last thing in the table section is a “Remove media” or “Clear media” button. Note: This is currently being added as stream of another bit of work.

I should have some time in the coming days to put together some higher fidelity designs, but hopefully in the meantime these should provide a path to start down. Let me know if there are any questions or concerns!

@ghost
Copy link

ghost commented Sep 20, 2020

@iamthomasbishop thank you for all the helpful wireframes and information. I had a few clarifying questions if you have time.

In terms of placement, I personally think we might want to reconsider where we place its table cell, but more on that later.

Were you hoping to modify the location of the fixed background switch within this work or at a later time? If now, where did you envision it being placed?

If an image or video is applied, it would have an edit button over the top right of it, in the same way that we show this button on Image blocks and soon Cover blocks.

When a user taps the edit button, is the intention that the bottom menu for Edit/Replace/Clear Media would slide in atop the media settings bottom sheet that is already open?

IMG_5120B3F525E7-1

I should have some time in the coming days to put together some higher fidelity designs, but hopefully in the meantime these should provide a path to start down.

I believe I have enough to begin work without a final design. The only design aspect I can think of currently I may be blocked by is icons for the left/right and top/bottom arrows you have in the X/Y axis sliders, potentially also an icon for the focal point hint crosshairs. Do these icons already exist in this repository? If not, are you able to provide assets for those icons?

Thanks!

@iamthomasbishop
Copy link
Contributor

@shadow351 Apologies for the delay 👋

Were you hoping to modify the location of the fixed background switch within this work or at a later time? If now, where did you envision it being placed?

I think if we can bring it into a consolidated media section as shown above (in this sketch in particular), that would be ideal.

When a user taps the edit button, is the intention that the bottom menu for Edit/Replace/Clear Media would slide in atop the media settings bottom sheet that is already open?

I believe so. And FYI that media-edit button didn't exist on the Cover block when I created those sketches — we just recently added it 😄

The only design aspect I can think of currently I may be blocked by is icons for the left/right and top/bottom arrows you have in the X/Y axis sliders, potentially also an icon for the focal point hint crosshairs

I can provide those horizontal/vertical arrow icons, as I don't believe they exist in the icon set. For now, let's just roll without icons and I will spin those up this week when I get a chance.

We do already have a + icon in the library, so can we use that as a start and see if we need to add a variation? In terms of the circle, perhaps we could create the circle programmatically, sized to about ~44-50px in diameter, so we don't need to rely on additional assets for that. Does that work?

@ghost
Copy link

ghost commented Sep 30, 2020

@iamthomasbishop thanks!

Were you hoping to modify the location of the fixed background switch within this work or at a later time? If now, where did you envision it being placed?

I think if we can bring it into a consolidated media section as shown above (in this sketch in particular), that would be ideal.

Ah, OK. Thanks for clarifying.

When a user taps the edit button, is the intention that the bottom menu for Edit/Replace/Clear Media would slide in atop the media settings bottom sheet that is already open?

I believe so. And FYI that media-edit button didn't exist on the Cover block when I created those sketches — we just recently added it 😄

Sounds good.

The only design aspect I can think of currently I may be blocked by is icons for the left/right and top/bottom arrows you have in the X/Y axis sliders, potentially also an icon for the focal point hint crosshairs

I can provide those horizontal/vertical arrow icons, as I don't believe they exist in the icon set. For now, let's just roll without icons and I will spin those up this week when I get a chance.

We do already have a + icon in the library, so can we use that as a start and see if we need to add a variation? In terms of the circle, perhaps we could create the circle programmatically, sized to about ~44-50px in diameter, so we don't need to rely on additional assets for that. Does that work?

Yeah, I believe that approach will work well. I'll give the + icon plus a programmatically drawn circle a try.

@ghost
Copy link

ghost commented Oct 5, 2020

  • Fixed Background (switch:) Below the “edit focal point” button would be a Switch to toggle the “Fixed background” setting. This would have the same affect as is seen on the web.

@iamthomasbishop the "Fixed background" switch is not currently available in the mobile app. Adding this switch to the mobile app is obviously included in the scope of this work, as you have clearly outlined.

That said, I noticed that if one enables "Fixed background" via the web interface today, it does not cause the Cover Block media to render with a parallax effect in the mobile app. Is the intention to add the parallax rendering effect with this work or merely add the switch to modify the "Fixed background" setting?

For context, the web parallax effect appears to be accomplished via a few CSS attributes. From researching this briefly, accomplishing the same effect in React Native for mobile is definitely doable, but it may require a bit more work to synchronize the image's positioning with the current position of the parent scroll view.

Please let me know your thoughts on including this feature. Thanks!

@ghost
Copy link

ghost commented Oct 6, 2020

@iamthomasbishop another question regarding this image. Did you intend for us to implement a tooltip to display "Drag to adjust focal point" above the focal point drag handler or was that merely guidance on how the UI should function? If the former, when should the tool tip display and when should it hide? Thanks!

@iamthomasbishop
Copy link
Contributor

Is the intention to add the parallax rendering effect with this work or merely add the switch to modify the "Fixed background" setting?

@shadow351 Great question. For the purposes of this iteration, I think we can focus on enabling support for the control. I think this is a safe bet because it matches the behavior of the mobile web editor, where you can toggle to control but the background position is static when scrolling inside the editor.

Did you intend for us to implement a tooltip to display "Drag to adjust focal point" above the focal point drag handler or was that merely guidance on how the UI should function?

I was hoping that we could display a tooltip, if it's not too much of a hassle. In this case, we could use the same timing and display logic that we're using for tooltips on the current starter page templates UI. // @geriux would you mind pointing him in the right direction? I can't remember the timing/display logic off the top of my head but I know you implemented tooltips in the SPTs context, so maybe you can help 🙂

@geriux
Copy link
Contributor

geriux commented Oct 9, 2020

I was hoping that we could display a tooltip, if it's not too much of a hassle. In this case, we could use the same timing and display logic that we're using for tooltips on the current starter page templates UI. // @geriux would you mind pointing him in the right direction? I can't remember the timing/display logic off the top of my head but I know you implemented tooltips in the SPTs context, so maybe you can help 🙂

Of course, so @shadow351 we have a very simple Tooltip component that was created for the templates selector, you can check it by using the WordPress app then going to Pages > Add a new one, you'll see at the bottom a tooltip (that will only be shown once FYI).

So I'm thinking we can update this component a bit to make it more reusable. What do you think @shadow351?

@ghost
Copy link

ghost commented Oct 9, 2020

So I'm thinking we can update this component a bit to make it more reusable. What do you think @shadow351?

Sounds like a good approach. I'll check out the Tooltip component and see how I can repurpose it for this feature. Thank you for the guidance.

@ghost
Copy link

ghost commented Oct 13, 2020

@iamthomasbishop should the tooltip attached to the focal point drag handle only be displayed once for the user or should it display each time?

@iamthomasbishop
Copy link
Contributor

should the tooltip attached to the focal point drag handle only be displayed once for the user or should it display each time?

@shadow351 I think only once, then it is dismissed when the user taps anywhere, I believe. Unless I'm missing something, I think that's the logic we followed for the tooltips on Starter Page Template selection.

@ghost
Copy link

ghost commented Oct 14, 2020

@shadow351 I think only once, then it is dismissed when the user taps anywhere, I believe. Unless I'm missing something, I think that's the logic we followed for the tooltips on Starter Page Template selection.

@iamthomasbishop alright. Once for a user makes sense. I figured that was the case.

In regards to dismissing the tooltip, I currently have it set to remain until a user interacts with the image drag area specifically. It sounds like you would prefer the tooltip to dismiss when a user taps anywhere on the screen, correct? This would match @geriux's usage for page templates. However, it does mean that it will take two taps for a user to modify the focal point, once to dismiss the tooltip and once to interact with the drag handle, slider, etc. Is that expected?

@iamthomasbishop
Copy link
Contributor

@shadow351

In regards to dismissing the tooltip, I currently have it set to remain until a user interacts with the image drag area specifically. It sounds like you would prefer the tooltip to dismiss when a user taps anywhere on the screen, correct?

That's correct.

However, it does mean that it will take two taps for a user to modify the focal point, once to dismiss the tooltip and once to interact with the drag handle, slider, etc. Is that expected?

I would expect both would happen simultaneously. For example: if the tooltip is visible and I tap-and-drag on a Slider handle, I would expect the Tooltip to dismiss immediately when I start the tap-and-drag gesture on the drag handle.

@ghost
Copy link

ghost commented Oct 21, 2020

However, it does mean that it will take two taps for a user to modify the focal point, once to dismiss the tooltip and once to interact with the drag handle, slider, etc. Is that expected?

I would expect both would happen simultaneously. For example: if the tooltip is visible and I tap-and-drag on a Slider handle, I would expect the Tooltip to dismiss immediately when I start the tap-and-drag gesture on the drag handle.

@iamthomasbishop alright, I will explore dismissing the tooltip while allowing interacting with the rest of the UI simultaneously. For awareness, this is not how the page template picker tooltip functions today. It requires two taps to dismiss and then interact with the remaining UI. I'm not sure if we want to change that as well at some point. cc/ @geriux

page-template-tooltip-dismiss

@ghost ghost mentioned this issue Oct 21, 2020
2 tasks
@iamthomasbishop
Copy link
Contributor

For awareness, this is not how the page template picker tooltip functions today. It requires two taps to dismiss and then interact with the remaining UI. I'm not sure if we want to change that as well at some point

@shadow351 Ah, my bad — I should've double-checked the tooltip flow. I do think my previous comment on our discussion here stands.

We'll be moving away from the Tooltip on that template selection UI soon, so I wouldn't worry about making any adjustments on that particular UI — however, if we can systemize the default component behavior in a way that the other instance inherits it, no complaints here. 😄

@ghost
Copy link

ghost commented Oct 23, 2020

@iamthomasbishop I was concerned about accomplishing the requested "dismiss on tap anywhere, but retain all interactivity" experience for the tooltip. I was concerned it may take a lot of effort or may not be possible with React Native's gesture responder APIs.

After some initial research, that concern still remains. I plan to research it some more, but I wanted to get your thoughts on an alternative UX that we could either (A) move forward with now to save time or (B) use as a backup plan if I am unable to accomplish your original functionality.

  1. The tooltip shows up the first time a user opens the focal point picker. Once shown, the flag is set so that the tooltip is never shown again.
  2. If a user specifically interacts with the drag handle, the tooltip disappears.
  3. If a user modifies the focal point coordinates via the range sliders, the tooltip disappears.
  4. If a user interacts anywhere else within the focal point picker (e.g. scrolls, taps outside controls), the tooltip remains visible.
  5. If a user closes and revisits the focal point picker, the tooltip will not display because the flag was set the first time the tooltip was displayed.

What do you think about this proposed UX? Would you prefer to execute it now, save it as a backup plan, or take a different approach?

Thanks!

@iamthomasbishop
Copy link
Contributor

@shadow351 Ok, thanks for the info! I'd like to try a test build if possible, because it's a bit hard to visualize the current state — would you be able to spin up a test build (iOS is preferred but either works)?

The approach you outlined seems pretty sound in theory, but as mentioned it's a little bit hard to say for sure without seeing a build first. One point in particular I'm not clear on:

If a user interacts anywhere else within the focal point picker (e.g. scrolls, taps outside controls), the tooltip remains visible.

In terms of what approach I'd like to take, I'd obviously prefer to smooth out any rough edges prior to shipping, but the stakes with this particular component/interaction are quite low, so I generally lean towards "ship and iterate" unless there are obvious bits of friction while testing a build.

@ghost
Copy link

ghost commented Oct 24, 2020

@shadow351 Ok, thanks for the info! I'd like to try a test build if possible, because it's a bit hard to visualize the current state — would you be able to spin up a test build (iOS is preferred but either works)?

@iamthomasbishop fair enough. I have not created a test build before, but I'll inquire with @geriux on the possibility of doing so.

The approach you outlined seems pretty sound in theory, but as mentioned it's a little bit hard to say for sure without seeing a build first. One point in particular I'm not clear on:

If a user interacts anywhere else within the focal point picker (e.g. scrolls, taps outside controls), the tooltip remains visible.

I meant to say that the only way to cause the tooltip to dismiss would be:

  • Dragging the focal picker handle.
  • Tapping somewhere on the media preview.
  • Dragging the range slider handle.
  • Modifying the range slider text input.
  • Closing the focal point picker bottom sheet.

All other interactions with the bottom sheet (e.g. scrolling, tapping somewhere that is not listed above) would not cause the tooltip to dismiss.

In terms of what approach I'd like to take, I'd obviously prefer to smooth out any rough edges prior to shipping, but the stakes with this particular component/interaction are quite low, so I generally lean towards "ship and iterate" unless there are obvious bits of friction while testing a build.

Alright. I suppose I will build out my proposed UX. Hopefully it will be a good enough UX to ship, but I'll let you make the final call. Thanks.

@geriux
Copy link
Contributor

geriux commented Oct 25, 2020

I have not created a test build before, but I'll inquire with @geriux on the possibility of doing so.

Of course, first, you need to make the bundles in your Gutenberg Mobile branch running npm run bundle after that you'll need to commit those changes and then update the commit reference in WP-iOS (and run rake dependencies) / Android. For Android, it'll make an apk automatically once you push your changes, for iOS, there will be an option in your PR to run the job to make the iOS build.

Let me know if you have any issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment