wordpress-mobile / gutenberg-mobile

Mobile version of Gutenberg - native iOS and Android
GNU General Public License v2.0
254 stars 56 forks source link

Button block - 1st iteration #1513

Closed pinarol closed 4 years ago

pinarol commented 5 years ago

We need to add support for Button block on the mobile side. It is highly used in pages and it would help us add variety on our starter page templates.

Let discuss which items would make a good first iteration, here are my thoughts but we can drop/add items:

Screen Shot 2019-10-29 at 18 52 00

The tricky part here will be the gradient colors, we'll need to add a new library to handle this. I am actually willing to leave gradients for a later iteration but not sure how to display a button that was created on web and already using gradient colors. If there's a good idea that can cover this we can leave it to the 2nd iteration.

Screen Shot 2019-10-29 at 18 52 11 Screen Shot 2019-10-29 at 18 55 34 Screen Shot 2019-10-29 at 18 58 06 Screen Shot 2019-10-29 at 18 56 37 Screen Shot 2019-10-29 at 19 00 34
pinarol commented 4 years ago

Update for shippable iterations:

1st

2nd

3rd

pinarol commented 4 years ago

We are working on opening the endpoint that will provide us the color codes from active theme, which will enable us to display the Button block with the correct color. But that solution will only land in 3-4 months if we are lucky because WordPress release cycles are taking that long, and it'll also take some more time for self hosted sites to update to this version. What is more, it is not %100 guaranteed that our endpoint will get approval, I am optimistic but I've never requested such a change before. So, in the meantime, I was hoping to come up with a kind of fallback mechanism that can unlock the Button bloc since most of the work that is required for the 1st iteration is already done.

First of all the editor supports a set of colors by default shown here. We can start with giving support for them on mobile(by just defining the same list on our side), what this means is if the backgroundColor of Button block is set to pale-pink we can resolve that to '#f78da7'.

However, for cases where we can't resolve the colors like backgroundColor="primary" we still need a fallback mechanism. How about using a default color but showing an indication or a kind of info message about Styling of this block may vary on web depending on the Theme used by the site? We can decide when we should show this message so we wouldn't need to show it if we are able to resolve the color.

@iamthomasbishop How does this sound to you?

ps1: BTW I am talking about the case where Button block was already created on Web, we wouldn't have this problem if user chose the color on mobile but for the first iteration we don't show color pickers.

ps2: Ideally color pickers are also shown with theme colors. And that is another case we'll need this new endpoint for. But for that case, we can at least display a palette with a default set of colors until the endpoint is ready.

iamthomasbishop commented 4 years ago

@pinarol I think this makes sense, but I have some caveats/questions. I was going to suggest we simply remove color completely (display on the block and in the inspector) until we can display the colors, but if we can provide a set of defaults (non-theme ones, if I'm understanding correctly) then that's slightly better.

I have a question – what happens in a case like this:

This could be potentially frustrating, so maybe it'd make more sense to strip out color customization until we can resolve the issue at hand? Or perhaps this might be a solution:

How about using a default color but showing an indication or a kind of info message about Styling of this block may vary on web depending on the Theme used by the site?

I think we could/should show the defaults but append the additional color I mentioned in my last point (if that's possible).

pinarol commented 4 years ago

I have a question – what happens in a case like this:

I create a button block on the web, select Theme Color A as the background I open the same post on mobile,

The problem starts here, we don’t know what color ‘Theme Color A’ is on mobile. So we don’t know how to display the block even in a read only way.

change the background to Standard Color X (one of our "defaults") I want to set it back to Theme Color A in mobile

Let’s say we used a default color ‘blue’ whenever we aren’t able to resolve the real color. It is possible to restore ‘Theme Color A’ with a kind of reset button that can be placed in the palette but when user taps reset they will see ‘blue’ color, in the html representation of the block ‘Theme Color A’ will be restored successfully with no problem.

Same thing is possible by showing the current color option in the palette instead of reset button, but again, the color shown in palette will be ‘blue’(the default color we chose).

iamthomasbishop commented 4 years ago

So we don’t know how to display the block even in a read only way.

Shoot 😞

Let’s say we used a default color ‘blue’ whenever we aren’t able to resolve the real color.

I would say let's use a completely neutral color for the fallback – some sort of gray.

It is possible to restore ‘Theme Color A’ with a kind of reset button that can be placed in the palette but when user taps reset they will see ‘blue’ color,

I don't think we should do this, I'd rather display a completely neutral color than an approximation – even if the color is appropriately applied in the HTML, this would be confusing because you're not seeing the actual color the button is being changed to.

pinarol commented 4 years ago

I don't think we should do this, I'd rather display a completely neutral color than an approximation – even if the color is appropriately applied in the HTML, this would be confusing because you're not seeing the actual color the button is being changed to.

Well, i used color blue just for being able to explain the scenario better. We should go with the color which you think is the most appropriate. I have no idea honestly what a ‘completely neutral color’ is :) that one is all your call.

Returning back to my suggestion about showing a kind of indication that the block can be displayed differently depending of the Theme site uses. What do you think about it? Thinking out loud, maybe we can put an info icon on the top right and when it is tapped we can open the bottom sheet with a warning similar to “Styling of this block may vary on web depending on the Theme used by the site”. We don’t need to show the info icon if the color was resolved successfully with the default set in hand, we’d only need this when color definition is a custom one coming from theme. And not every theme is defining custom ones, my premium site is one example. Just throwing an idea...

lukewalczak commented 4 years ago

As @pinarol has mentioned currently we are not able to manage saving/receiving the backgroundColor between platforms (except the custom one chosen from picker or gradient). It seems to me that we don't have a workaround for that 😢.

Actually setting a fallback color once the user set backgroundColor from palette along with some kind of information makes sense for me (will need some kind of mock for that if we want to proceed with that idea / maybe some kind of dismissable dropdown alert/ snackbar after adding button 🤔)

The problem with backgroundColor will stop also work over the 2nd iteration (hopefully we'll be able to go with the 3rd, so we can change the order), but it's good to ship at least the 1st one and let users work with Button component, even if some features are missing.

iamthomasbishop commented 4 years ago

Returning back to my suggestion about showing a kind of indication that the block can be displayed differently depending of the Theme site uses. What do you think about it?

Thinking out loud, maybe we can put an info icon on the top right and when it is tapped we can open the bottom sheet with a warning similar to “Styling of this block may vary on web depending on the Theme used by the site”.

@wppinar Something similar to what we do on bottom sheets makes sense. I like your suggestion.

lukewalczak commented 4 years ago

@iamthomasbishop could you please provide some kind of mock for that? 🙏

iamthomasbishop commented 4 years ago

As discussed in Slack, here's a mock of the flow:

IMG_0683

pinarol commented 4 years ago

I have some hesitations about the info message, both its place and its content. Because I think it is not meeting the real need here. We need the message because we are missing the endpoint to resolve some color codes coming from theme(they come in with the already created blocks on web). However, we can have a color picker with a default set of colors even if the new endpoint is not ready to use yet(people won't have it until they update their WordPress version even if it is merged). In this case, we'll still won't be able to resolve some color codes set on web, so, we'll still need the message to communicate that. But this way, the message will be lost once we add the color picker. Does that make sense? @iamthomasbishop

Same thing is correct for other blocks that lean on some theme colors so this can be used by other blocks as well. I don't have a strong opinion about what we should show instead, but technically we know exactly when we are not able to resolve the color. A few options that pop into my mind is:

  1. Just having an inline message in the settings whenever we aren't able to resolve.
  2. Or showing this message by tapping a kind of "info" icon on one of the corners of the block, or, on the inner toolbar(we can show the icon only when we aren't able to resolve):
Screen Shot 2020-01-06 at 15 35 45
lukewalczak commented 4 years ago

Another problem with a current solution is a Keyboard which is appearing for a second between bottom sheet switches (BottomSheet with controls -> keyboard -> BottomSheet with a message about missing colors implementation). It happens on iOS because closing the modal restores the focus on RichText automatically, which results in keyboard opening. keyb

iamthomasbishop commented 4 years ago

@pinarol Thanks for the ping — forgive me if I'm not understanding perfectly, but I mostly think I see what you're saying. I think your suggestion to use footer text on the table row wouldn't be bad although I would have to see it in practice to see how it feels.

I sketched out some ideas, to make sure I'm on the same page. On the left is our current solution — I think you're right this probably won't be ideal soon. I would suggest we go with one of the following:

IMG_0700

We could use either of these approaches in the same way after adding the swatches in.

But this way, the message will be lost once we add the color picker. Does that make sense?

Can you clarify what you mean by "will be lost"? I expect we'd have to adjust the copy slightly when we change the UI, but I'm not exactly sure what you mean.

pinarol commented 4 years ago

@iamthomasbishop Proposal A would solve the problem about the thing I was mentioning by "will be lost". Because once we have color palette we were going to need to move the entry point of the message(currently ? icon) to somewhere else.

However, this time the message won't be suitable anymore:

Because once we have palette available it will become invalid to say "Color settings are not yet supported". And the color mismatch problem between the editor and the output will continue even though we have a color palette(because of the missing endpoint). What makes things even more complex is, sometimes, we do render the Button with correct color because we have a default set of colors defined on client(even without the endpoint), in this case we don't need to show a message at all. That's why I think Proposal B fits the logic better because the problem we are having here isn't directly related with having a color palette.

For that reason, I think the message should be a bit different like: "We are still working on making the colors synced with the Theme used by the site. In this period please check the preview to make sure you are getting the results you want." I might not have chosen the perfect wording so I appreciate any fixes.

Another advantage of going with Proposal B is this problem @lukewalczak mentioned. iOS tries to focus back to the lastly focused text input right after a modal is closed, so opening a modal from another modal is currently problematic because it opens the keyboard in between, it happens quick, so it looks like a flashing.

Wdyt?

iamthomasbishop commented 4 years ago

However, this time the message won't be suitable anymore:

Oh, certainly — the messaging will need to be updated based on the state of the UI either way, so either of these options would work. One of the main reasons I don't love the idea of using the footer text is that it could end up being a long-ish message and take up lots of space, so tucking it in the sheet makes more sense to me. If we can keep the messaging short and clear, then I think that's fine.

If we go with footer messaging, how about something shorter like: "Theme colors aren't available at this time." I'm not sure if we need to, but we could also mention previewing — just not sure how much value it adds.

iamthomasbishop commented 4 years ago

@pinarol One more question: I understand we won't have theme-defined colors yet, but here's my concern. If I open a post that I created on the web that has a button block with a custom color, let's say with "Color A" — when I open this post, is that button displayed as it was created on the web (using Color A)?

My concern is that the user won't be able to "get back" to that Color A if they've changed to a different color. If we can save/pre-pend "Color A" to the list and append our other colors (and a custom picker) I think that is probably okay until we have theme-defined color support. Sort of like a pseudo-undo button to get you back to what it was when you opened the post.

pinarol commented 4 years ago

@iamthomasbishop

If we go with footer messaging, how about something shorter like: "Theme colors aren't available at this time." I'm not sure if we need to, but we could also mention previewing — just not sure how much value it adds.

"Theme colors aren't available at this time." Sounds totally good 👍 Should we show this only when the color can not be resolved on mobile, or should we show this every time?

If I open a post that I created on the web that has a button block with a custom color, let's say with "Color A" — when I open this post, is that button displayed as it was created on the web (using Color A)?

Right, if the user taps "Custom color" link then the color is stored as "#2e5e72" in the html, so we'll have no problem displaying it on mobile.

y concern is that the user won't be able to "get back" to that Color A if they've changed to a different color. If we can save/pre-pend "Color A" to the list and append our other colors (and a custom picker) I think that is probably okay until we have theme-defined color support. Sort of like a pseudo-undo button to get you back to what it was when you opened the post.

"save/pre-pend "Color A" to the list and append our other colors" 👈 yes, this should be possible. I see a "Clear" button on web's palette to revert the changes, that can also be an option. @lukewalczak please correct me if you have any concerns.

iamthomasbishop commented 4 years ago

"Theme colors aren't available at this time." Sounds totally good 👍 Should we show this only when the color can not be resolved on mobile, or should we show this every time?

I think it should be displayed in one place, whether that's shown as footer text in a single place or behind a (?) icon button.

"save/pre-pend "Color A" to the list and append our other colors" 👈 yes, this should be possible. I see a "Clear" button on web's palette to revert the changes, that can also be an option. @lukewalczak please correct me if you have any concerns.

Awesome, I think that's a good start then.

pinarol commented 4 years ago

I think it should be displayed in one place, whether that's shown as footer text in a single place or behind a (?) icon button.

Actually I was trying to ask about the pre-conditions for displaying this message. If a custom color was set on web then we have no problem displaying the same color on mobile. In this case, should we still show this message?

Regarding place, I vote for going with footer text because opening modal from another modal can be problematic on iOS and that kind of problems usually have ugly solutions since the original behavior is controlled by the system, not by our code.

iamthomasbishop commented 4 years ago

Actually I was trying to ask about the pre-conditions for displaying this message. If a custom color was set on web then we have no problem displaying the same color on mobile. In this case, should we still show this message?

Ahhh, okay, thanks for clarifying. I do think we should still show it in that case, yes. We could tweak the message to "Some theme colors may not be available at this time."

Regarding place, I vote for going with footer text because opening modal from another modal can be problematic on iOS and that kind of problems usually have ugly solutions since the original behavior is controlled by the system, not by our code.

Sounds good 👍 Can we add the footer text to see how it looks?

lukewalczak commented 4 years ago

Thanks for your input there, appreciate it!

Partially like the idea of discarding additional bottom sheet since it's a bit buggy on ios. Messages sound cool for me 👍

Sounds good 👍 Can we add the footer text to see how it looks?

Actually, I wanted to ask how it should look like? Can you @iamthomasbishop please post a quick/rough mock for it? 🙏

save/pre-pend "Color A" to the list and append our other colors" 👈 yes, this should be possible. I see a "Clear" button on web's palette to revert the changes, that can also be an option.

Need to investigate it! Will update you on Monday in that matter!

iamthomasbishop commented 4 years ago

Actually, I wanted to ask how it should look like? Can you @iamthomasbishop please post a quick/rough mock for it? 🙏

Ignore the swatches themselves, but for the footer, something like this maybe?

image
lukewalczak commented 4 years ago

My concern is that the user won't be able to "get back" to that Color A if they've changed to a different color. If we can save/pre-pend "Color A" to the list and append our other colors (and a custom picker) I think that is probably okay until we have theme-defined color support. Sort of like a pseudo-undo button to get you back to what it was when you opened the post.

I did a quick investigation in that matter and there are results:

  1. Actually, on the web there is a Clear button, but it's resetting to the default color instead resetting to the color which was set previously:

reset_bg_web

  1. We can store an initial background color and then after some color changes easily reset to it:
lukewalczak commented 4 years ago

@iamthomasbishop Could you please tell me also if we should keep the Coming Soon message along with footer? Please check the screenshot:

Screenshot 2020-01-13 at 14 24 36
iamthomasbishop commented 4 years ago

Actually, on the web there is a Clear button, but it's resetting to the default color instead resetting to the color which was set previously:

Yea, I think the "clear" button label is confusing, tbh — because "clear" to me says "a clear background". If we do have a button w/ this functionality on mobile, we could change the label to "reset". I think a previous iteration of the button block proposed this but we ended up removing.

We can store an initial background color and then after some color changes easily reset to it:

Thinking about this issue again, I think just having the undo/redo buttons may be enough for the "get back" use case. I can simply tap undo until I am "back" to the desired color.

Could you please tell me also if we should keep the Coming Soon message along with footer? Please check the screenshot:

Perhaps we should remove the "coming soon" row label and substitute the footer text. But change it slightly to say "Theme colors are not available at this time." Thoughts @lukewalczak @pinarol ?

pinarol commented 4 years ago

Perhaps we should remove the "coming soon" row label and substitute the footer text. But change it slightly to say "Theme colors are not available at this time." Thoughts @lukewalczak @pinarol ?

Sounds better 👍

lukewalczak commented 4 years ago

Sounds good for me as well. Is it correct right now (screenshot)?

Screenshot 2020-01-14 at 13 27 15
iamthomasbishop commented 4 years ago

@lukewalczak This looks pretty close, but I imagined we’d want to use the footer note style, like your example above.

lukewalczak commented 4 years ago

Ok, sounds like I have the final solution 😅 :

Screenshot 2020-01-14 at 18 04 13

@iamthomasbishop What should be the color of footer text?

iamthomasbishop commented 4 years ago

Good question. Footer text can probably be the same color as the section title (“Color Settings” label).