cryptee / web-client

Cryptee's web client source code for all platforms.
https://crypt.ee
Other
449 stars 23 forks source link

[Bugs] Assorted layout issues #105

Closed dhda closed 2 years ago

dhda commented 3 years ago

Describe the bug A handful of small non-blocking layout issues.

In full screen windows like the table of contents and summon ghost folder windows, the main content of the window is tiny and painful to use.

Text in the new document modal is very squashed together and overlapping.

Orientation changes cause a non-visible off-screen yellow warning dialog to flash on the screen.

Screenshots Visible content is tiny in full screen windows: Image 2 IMG_6698

New document modal: IMG_6692

Orientation changes: Image

System Information (please complete the following information):

johnozbay commented 3 years ago

Hey there! Thanks a lot for these! Super helpful stuff!

I'm now realizing the reason why we messed up this way has to do with our simulators. For some reason all our simulators, including Safari itself in responsive design mode thinks iPhone 6, 7 & 8 have 375px width.

image

Whereas in reality what I'm seeing from your screenshots is that it's actually 320px wide. Hence the reason why we missed all these and our automated tests all passed. I'm going to have to change a bunch of things + our build / test system with custom resolutions to address these. So making a little checklist to keep this thread updated.

The new document modal is an easy CSS fix, so I'm pushing this out now, but for the other two issues I'll have to do a bunch of tests, especially with devices that have notches, since once rotated, the notch will cover the left sidebar. And I can't actually apply some of these without extended on-device testing, since tablets don't have notches but some phones do, and some android devices have literal holes on their displays. i fucking hate notches and holes 🤦🏻‍♂️

Thanks for these ✌🏻 Deeply appreciate all the help!

dhda commented 3 years ago

The 6s’s resolution is supposed to be 750x1334 (375x667), so that’s super weird that the screenshots are a different resolution and very slightly different aspect ratio. I confirmed that the screenshots are 640x1136 directly on-device.

Edit: Oh! I think what may be going on is that I have my device set to display “Zoomed” (Settings > Display & Brightness > Display Zoom > Zoomed) which scales up all UI elements (apparently by simulating a different screen resolution). In Standard zoom Cryptee’s v3 design definitely makes more sense (though the content in full screen modals is still pretty tiny). #99 especially makes it almost unusable on the Zoomed setting.

Screenshots come out at the expected resolution on Standard zoom, too.

It’s really really interesting, going back to Standard zoom seems to have resolved some display lagginess issues I’ve noticed on this phone. It makes me think Apple is probably implementing the Zoomed mode by rendering to a buffer that has a bigger size than the pixel resolution of the physical display and then downsampling it to the screen resolution (the same way they handle the scaled resolution options for MacBook retina displays when the scaled virtual resolution isn’t an integer factor of the display resolution).

johnozbay commented 3 years ago

Hahahah wow! 🤯

Discovered and learned a lot thanks to this thread and can confirm some of these are only issues in zoomed-mode. And yeah looks like it indeed makes things behave strangely / cause performance issues in rare situations. 🤯 i.e. presumably due to all the scaling etc going on, our image optimization speeds seem to take a tiny speed hit as well.

I don't know how logical it is for us to keep designing for a resolution that's smaller than the smallest phone's screen size, (i.e. 320px wide vs 375px) that being said, it's a small difference, and an easy number to keep in mind.

I will still update our testing to check for 320px, so that things still look nice and sharp on zoomed mode as well when possible.

Thanks a lot for these fun discoveries hahah

dhda commented 3 years ago

The 1st generation iPhone SE (still supported by Apple!) actually has the smallest iPhone screen size, and it happens to be exactly the same resolution as the Zoomed 6s at 640x1136px.

I hope you do keep supporting the Zoomed display modes across devices. It can be an important accessibility feature for people with eyesight issues (personally it helps me with eye strain).

johnozbay commented 3 years ago

We will continue to test against 320px and do our best to support 320px as best as we can, but we cannot support devices like iPhone 5S anymore unfortunately. The problem is that webkit / safari on older iOS devices are forever stuck on an old version, due to Safari updates being tied to OS updates on iOS for no reason at all. thx apple 🤦🏻‍♂️

Not just because it's a security nightmare, but also it's lacking so many features we now use in v3 to speed things up.

Part of what made the V3 speed boost possible was that we don't fallback to slow polyfills for a bunch of features anymore. We realized that trying to make things work for 5+ year old iOS devices was part of what made the app slow for everyone. (i.e. even on ancient android phones you can still get the latest chrome) but on older iOS devices you cannot get a newer webkit engine, making the devices ridiculously insecure and outdated in terms of web standards.

dhda commented 3 years ago

I’m confused, I think that 1st gen SEs can still update to iOS 14.

johnozbay commented 3 years ago

Whoops my bad – I've been thinking about iPhone 5S (and not the SE) and yes, as long as it runs iOS 14 it's all good and we'll support it. Evidently I'm under-caffeinated, and need more coffee! ☕️

My point being, we do and will continue to test against 320px. 😅

johnozbay commented 3 years ago

Also to clarify what I meant by updating our testing to check for 320px:

We design / test / aim for 320px minimum, and test against this as much as we can. That being said, our automated tests run on "iPhone 6/7/8" and their resolution in the testbench we use is 375px. So I'm in the process of updating our automated tests to use both 320px and 375 on iPhone 6/7/8 now (to also accommodate for zoomed-mode)

Hoping this makes more sense 😅

johnozbay commented 3 years ago

And to add one more point – the reason why our automated tests missed the first one is because I think the zoomed-mode doesn't just zoom in, but also modifies the way fonts are rendered. Since we use a rem-based layout, and variable fonts, some of the stuff broke in zoomed-mode 320px that wouldn't normally break in non-zoomed-mode 320px etc. So there are a few different issues at play here causing the confusion, and hoping I could clear it now ha 😅

johnozbay commented 3 years ago

In fact, when designing components of the platform, we made sure all components are exactly 320px wide. So popups, for example are 320px (20rem) :

https://github.com/cryptee/web-client/blob/d83c0fc74ba6e85480f95f02715f4876fe9e66cb/source/css/global.css#L522-L531

Or modals, for example are also 320px (20 rem with padding) :

https://github.com/cryptee/web-client/blob/d83c0fc74ba6e85480f95f02715f4876fe9e66cb/source/css/global.css#L951-L961

Or the left sidebar in docs, also 320px etc

https://github.com/cryptee/web-client/blob/d83c0fc74ba6e85480f95f02715f4876fe9e66cb/source/css/docs.css#L77-L80

etc etc hoping this makes sense

johnozbay commented 3 years ago

Just released 8d79a670374faedde7630446a15e28f897594a99, and it has a couple of mini CSS additions to address orientation-change layout issues / making the app function a tad better in landscape mode on thin and tall devices, albeit not perfect. Just wanted to stop by to let you know we're still working on this. ✌🏻

johnozbay commented 2 years ago

Hi there,

Great news 🎉

https://github.com/cryptee/web-client/commit/13fec562257ea35b3fec08d5b3a9ec180e457f78 changes the way modals and popups are displayed entirely, and all issues on this thread, incl modals flashing while rotating etc should be all fixed now.

Thanks a ton for reporting these, all the screenshots and your patience with all this. We wouldn't be able to identify and fix as many issues as we did without your help and support!

Needless to say, please feel free to re-open this thread or file up new issues if you bump into other bugs like these. We'll tackle them as quickly as we can!

Best, J