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

[WPAndroid] inline toolbar not visible when on last block and inserting with Enter #427

Closed
mzorz opened this issue Dec 20, 2018 · 6 comments

Comments

@mzorz
Copy link
Contributor

mzorz commented Dec 20, 2018

When tapping on ENTER to get a new block and you're already at the bottom of the block list, the inline toolbar gets hidden behind the soft keyboard.

inlinetoolbarhidden

Tested on Pixel, Android 8.1

@mzorz mzorz added [Type] Bug Something isn't working Writing Flow labels Dec 20, 2018
@mzorz mzorz added this to the Alpha milestone Dec 20, 2018
@koke
Copy link
Member

koke commented Dec 21, 2018

@pinarol maybe the scrolling needs tweaking on Android?

@pinarol
Copy link
Contributor

pinarol commented Dec 21, 2018

@koke @mzorz one thing that I noticed is scrolling until we show the bottom end of the block isn't always the ideal solution because it results unwanted behavior on long texts. For example when you put the caret at the beginning of a long text you don't want it to get out of the viewable zone, but it does currently(on iOS), because it always tries to show the end of the focused block. This mechanism is built in KeyboardAwareScrollView and this will happen even when we don't include the inner block to the viewable area, it just focuses to the end of the react native element. So the ideal solution is I think is always trying to focus to the caret. Not sure how to do that currently but this might be a better thing to aim for considering the issues on long paragraphs. What do you think?

On the other hand on Android focusing to the cursor is all system managed (I think) and it behaves pretty different from iOS, maybe it behaves different from device to device also, not sure about it. I saw that Android can not focus at the END of a long text on some emulators( <- this may be something we try to fix instead.)

I am not sure if coming up with an application level solution is the way to go on Android, we can really use an Android dev opinion on this because it is hard for me to distinguish which part is handled on OS level and how customisable it is. I tried using KeyboardAwareScrollView on Android also btw. But it only handles the extra scroll on app level, so scrolling looks like it happens in 2 parts, first OS level scrolling occurs and then app level scrolling occurs and these are easily distinguishable from each other because there's a delay in between. So scrolling didn't look smooth.

@koke
Copy link
Member

koke commented Dec 21, 2018

Hmm, so we want the bottom to be visible for new blocks that are created on enter, but if you just tap on a partly visible block there's no need to do that (we might still want to scroll a bit so the cursor is visible). I'm thinking that instead of scrolling to the bottom, what we might want to do is scroll to the top plus the default height of the block, if that makes sense. I'd like to hear what @iamthomasbishop thinks about the scrolling after testing the alpha.

I don't know much about scrolling on Android either.

@iamthomasbishop
Copy link
Contributor

I'm not sure how I missed this, sorry for the delay!

I think this is already being worked on here, but just wanted to jot down my thoughts.

I would also hope that we can scroll a new block up just enough so that the bottom edge of the block is above the quick/formatting toolbar, but @pinarol's point about tall blocks makes sense.

instead of scrolling to the bottom, what we might want to do is scroll to the top plus the default height of the block, if that makes sense

I think it makes sense to scroll new blocks & "short" blocks to the edge, because then the Quick/Formatting Toolbar and this inline toolbar are both reachable and contextual. If we were to scroll to the top, the chance of the inline controls being reachable is less on new/short blocks.

Forgive my technical ignorance but perhaps there could be some logic that says:

  • if block is ≤ y, scroll the block to the edge (y being either a static px/pt height or relative to the available viewport height minus keyboard/navbar height)

  • if block is ≥ y, scroll to where cursor location (with maybe an extra ~24-56pt of buffer for breathing room?)

I had another idea, but ignore this for now unless it seems like an idea worth exploring at this time. I'm not sure if it is worth actioning on (or would even feel natural). I was thinking maybe we'd want to have the inline toolbar be "sticky" so that it's always visible when the focused block is partially-visible on screen (you'd probably want to hide it when it's off-screen under the keyboard or below?).

Hopefully that makes sense, and again great work @pinarol on this scroll stuff!

@pinarol
Copy link
Contributor

pinarol commented Jan 3, 2019

Thanks for comments @iamthomasbishop I already did some experiments similar to what you suggest, let me give some details.

if block is ≤ y, scroll the block to the edge (y being either a static px/pt height or relative to the available viewport height minus keyboard/navbar height)

I tried calculating this y dynamically, because it is not very easy to determine a constant y considering we have very little space on landscape mode in iPhones when keyboard is open, on the other hand we have lots of space on iPads even if we are on landscape mode. In some iPhones we don't have enough space to show the inner toolbar in landscape mode when keyboard is open in any case.

So I ended up calculating the distance between caret and the bottom of the block, I can scroll till the bottom of the block if both caret and the end of the block fit in the viewable area together. So it is basically as simple as scroll till the bottom if there's room. It sounded good very in theory, but had problems on practice. For long text fields while tapping on a certain line was resulting in scrolling to caret, tapping the line just below could be resulting in scrolling to the bottom of the block, so it looked really confusing, even I was confused while using because it didn't feel like it is behaving in an expected way. And then I came up with showing the inner toolbar only if caret is at the last line, we can play with this and set this to (last line - n). The reason I selected the last line is it felt more predictable. I am still using this dynamically calculated y to decide if we can scroll till the bottom of the block but it is not the single criteria currently.
At last I ended up with the below decision mechanism in my recent PR:

screen shot 2019-01-02 at 13 45 23

At this point I recommend everyone to try the new version for a while and see how it feels and then reach me out about the things that can be enhanced. It can be really challenging to decide things theoretically, I can also make a live session to experiment on things. Just let me know your thoughts once you get to try.

@SiobhyB
Copy link
Contributor

SiobhyB commented Nov 16, 2023

As part of the recent UX improvements, the buttons in the toolbar below each block were moved to the editor's main toolbar, which is always visible. I'll go ahead to close this issue, as it's effectively resolved as part of those changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done (keep clean, manually)
Development

No branches or pull requests

8 participants