200ok-ch / organice

An implementation of Org mode without the dependency of Emacs - built for mobile and desktop browsers
https://organice.200ok.ch/
GNU Affero General Public License v3.0
2.42k stars 149 forks source link

UX/Design Regression #454

Closed munen closed 4 years ago

munen commented 4 years ago

AutoFocus introduced a regression for iOS.

Instead of seeing the input field after clicking 'search' or 'todo list' like this

signal-2020-09-01-193914_003

The user sees this

signal-2020-09-01-193914_002 signal-2020-09-01-193914_001

munen commented 4 years ago

Update: The reason is that the drawer has a position: fixed with a height of '92%'.

Maybe it's possible to resize the Drawer component with something like onFocus={() => { chromeHeight = window.innerHeight}}.

Here's some good notes on auto-focus: https://blog.danieljohnson.io/react-ref-autofocus/

munen commented 4 years ago

@schoettl FYI, I've spent some more time on this issue and finally pushed a 'solution': https://github.com/200ok-ch/organice/pull/462

schoettl commented 4 years ago

Wow, thanks! Sad, that it doesn't work without this isIos()...

munen commented 4 years ago

Sad is that after some hours of research, the only option is to disable the otherwise very sensible default of autoFocus-_-

munen commented 4 years ago

Update: The discussed “fix” of disabling autoFocus for iOS worked in development. It doesn’t work when deployed (neither in Safari nor as PWA on the homescreen).

schoettl commented 4 years ago

Oh no... so isIos() doesn't work in production? Because it worked before we added the autoFocus property?

munen commented 4 years ago

Oh no... so isIos() doesn't work in production?

I think so. When running organice locally, with my latest change, I don't have autoFocus set, so it behaves as before: Click on Search or Todo List opens the drawer in 'close to full screen'. Then I can tap into the search field.

Now, with or without my change in production, when I click on Search or Todo List, it looks like in the screenshots in the description of this issue.

Because it worked before we added the autoFocus property?

Yes, it did. Which isn't to say that adding autoFocus isn't generally a good thing to do here. I think it is generally the right thing to do in this scenario.

munen commented 4 years ago

A different UX issue that I noticed with autoFocus is this: Sometimes (only sometimes), when I open Search or Todo List in a file that has few items, it'll look like this:

image

Tested on FF 80 on Debian Linux.

Not a big issue, but weird. Firstly, because it's only sometimes and secondly because it seems to only happen in smaller files.

munen commented 4 years ago

I'm trying with a more generalized approach to recognize iOS: 3695179

This still works for me when running it locally. :crossed_fingers: that it works when deployed^^

munen commented 4 years ago

This is very weird. Even when running a yarn build, it works. But when deployed, it doesn't. I cleared the cache, restarted the browser, had to re-login and the problem persists.

munen commented 4 years ago

I'm doing the hardcore thing and manually delete whatever we have on the FTP and re-deploy. Meaning organice.200ok.ch will shortly be offline. I can't really tell if that helps, but I don't understand why the prod build would work locally, but not when deployed.

munen commented 4 years ago

No dice.

munen commented 4 years ago

It get's weirder. I've just created https://staging.organice.200ok.ch/ which is the same, but shows an alert for isIos() when opening one of the problematic modals. They show true on my phone and then, the modal doesn't have autoFocus. But that might be, because the alert stands in the way.

schoettl commented 4 years ago

WTF... there is definitely no alert() in the code base that shows true...

munen commented 4 years ago

WTF... there is definitely no alert() in the code base that shows true...

I added alert(isIos()); to see if it returns false when deployed. It doesn't.

munen commented 4 years ago

Now for the weirdest part: I removed the alert, again. And on Staging, I don't have autoFocus on iOS. So it works as expected!

On prod, it still doesn't work. I cleared the cache, restarted the browser. I'll resort to clearing the cache, deleting the PWA and rebooting now.

munen commented 4 years ago

Still no dice!

This makes no sense.

munen commented 4 years ago

There must be some very weird iOS specific cache that remembers this autoFocus even though all caches are deleted.

schoettl commented 4 years ago

Weired...

... and now you have no devices left to test with a fresh cache? ^^ I don't have, can't help with testing...

munen commented 4 years ago

I have just deleted all caches and thereby lost all my history, open tabs and everything. Then I rebooted. And it still does not work on organice.200ok.ch, but it does work on staging.organice.200ok.ch.

Feels like alchemy.

munen commented 4 years ago

... and now you have no devices left to test with a fresh cache? ^^ I don't have, can't help with testing...

Yes, I do. I have an iPad mini. I just tried organice.200ok.ch - and it works. As it should.

But it will have to work on my iPhone, too. It would be pretty sad if I cannot used organice on my phone anymore. I have easily spent over half a day on this now. To get the status quo.

munen commented 4 years ago

Ok, so now I'm resorting to using an alias: org.200ok.ch

But, I've actually lost out a lot, because now iOS is punishing me with this:

FB1F8CA6-28A0-4F23-9997-7DC104CE3A12

munen commented 4 years ago

I can't use organice.200ok.ch anymore, because we pushed a broken feature once.

And I can't use a link to my file as the PWA anymore, because iOS changed something.

Gotta be way more careful with testing in the future.

munen commented 4 years ago

Also, this whole thing will stay broken for all iOS users who have used organice in the last week.

I tested on my wifes phone (who is also an organice user). So she'll have to use the same workaround and lose the same benefits as me.

Everyone else will just have a broken experience now. Forever. Or until we force a new domain. Or Apple fixes whatever crazyness they've invented to break this.

This is not a good spot to be in.

munen commented 4 years ago

Weirdly, it works for her now.

What in the ... is going on here.

munen commented 4 years ago

Closing this to end the misery.