Fix composer header hidden by mobile browser #2279

SychO9 · 2020-08-29T22:35:49Z

Fixes #1670

Changes proposed in this pull request:
Fixes the position of the composer to the screen, you can test this in https://beta.flarum.site, just make sure you add the relevant CSS from the admin section.

@media (max-width: 767px) {
    .Composer:not(.minimized) {
        position: fixed;
    }
}

Reviewers should focus on:
My only concern is that looking at git blame, the composer's position was made absolute in 2015 to fix a bug with iOs 8/9, but there doesn't seem to be any mention of what that bug exactly was, so I cannot tell if this could potentially bring back that bug.

Relevant commit: e6e2cdd

Screenshot
The issue has images of the bug fixed by this PR.

Confirmed

~~[ ] Frontend changes: tested on a local Flarum installation.~~
~~[ ] Backend changes: tests are green (run composer test).~~

askvortsov1 · 2020-08-29T23:30:26Z

less/forum/Composer.less

@@ -114,7 +114,7 @@
    background: @body-bg;

    &:not(.minimized) {
-      position: absolute;


My understanding is that this is intentional: we want people to be able to scroll up and down in a discussion while working on their reply. That being said, my understanding might be incorrect.

oh, if that's the case then we wouldn't want to change this then. From the commit I mentioned above e6e2cdd my understanding is that it was only done to fix a certain bug.

Also, users can always minimize the composer to take a look at the discussion, so I'm not sure if scrolling up and down from there is really needed

Not inherently opposed to changing, but let's revisit this in the next release cycle and hopefully have some discussion on it

stale · 2020-12-23T04:09:47Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum.
In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

zerosonesfun · 2021-01-03T13:54:11Z

Original solution above, using fixed, for me completely hides composer now.

This may be best. This ensures the composer is at the bottom, similar to how it is on desktop. It makes the most sense. Composer box adjacent to your virtual keyboard. "But, we want people to be able to scroll and see behind it," some say. 1) You can still scroll up and see other posts. 2) After typing enough content you start to see your new post behind the post box without scrolling. 3) There is the eyeball 👁 preview button. People should use that to see their post before posting. There is very limited real estate on mobile with the keyboard expanded. We should no longer play the “must see everything behind it” game when it comes to mobile.


@media (max-width: 767px) {
    .Composer:not(.minimized) {
      top: initial !important;
      bottom: 0 !important;
        
    }
}

Looks like this:

matteocontrini · 2021-01-03T13:57:34Z

@zerosonesfun I think there might be the risk that the composer gets hidden by the browser header on small screens, right?

SychO9 · 2021-01-03T14:16:50Z

@zerosonesfun did you only apply the LESS changes or did you also apply JS changes (i.e: all changes done by this PR) ?

zerosonesfun · 2021-01-03T14:25:44Z

@zerosonesfun did you only apply the LESS changes or did you also apply JS changes (i.e: all changes done by this PR) ?

Sorry, no I didn’t do a pull request. Just commenting that the original pull request probably won’t work.

zerosonesfun · 2021-01-03T14:32:40Z

@zerosonesfun I think there might be the risk that the composer gets hidden by the browser header on small screens, right?

Maybe. But, that’s a risk even if it is left as-is isn’t it? You could also make the composer height a little smaller. It’s close to 250px as-is. It could be changed to 200px or 220px.

Another way of looking at this is: Unless a better fix can be found relatively quickly... Should Flarum be a good experience on super tiny screens or a good experience on very popular iPhone screens? I know both... maybe eventually. But, the very popular iPhone is overlooked too much, IMO.

SychO9 · 2021-01-03T14:37:12Z

Sorry, no I didn’t do a pull request. Just commenting that the original pull request probably won’t work.

It's hard to tell if you haven't also applied the js changes if this is a bug with the PR or not:

Original solution above, using fixed, for me completely hides composer now.

This may be best. This ensures the composer is at the bottom, similar to how it is on desktop.

That looks nice on your mobile screen, but how does it behave on smaller mobile screens where there is no space for both the keyboard and the composer ? I imagine you wouldn't be able to see the title field anymore.

Also, fixed positioning fixes a problem where the composer is stuck at a specific position in the screen, creating issues when closing the keyboard and other scenarios: https://discuss.flarum.org/d/24831-ui-bug-mobile-postdiscussion-is-not-on-the-screen-sometime

Another way of looking at this is: Unless a better fix can be found relatively quickly.. Should Flarum be a good experience on super tiny screens or a good experience on very popular iPhone screens?

I don't understand what you mean, we are not forced to choose between both, this pull request fixes all issues.

zerosonesfun · 2021-01-03T14:48:28Z

Sorry, no I didn’t do a pull request. Just commenting that the original pull request probably won’t work.

It's hard to tell if you haven't also applied the js changes if this is a bug with the PR or not:

Original solution above, using fixed, for me completely hides composer now.

This may be best. This ensures the composer is at the bottom, similar to how it is on desktop.

That looks nice on your mobile screen, but how does it behave on smaller mobile screens where there is no space for both the keyboard and the composer ? I imagine you wouldn't be able to see the title field anymore.

Also, fixed positioning fixes a problem where the composer is stuck at a specific position in the screen, creating issues when closing the keyboard and other scenarios: https://discuss.flarum.org/d/24831-ui-bug-mobile-postdiscussion-is-not-on-the-screen-sometime

Another way of looking at this is: Unless a better fix can be found relatively quickly.. Should Flarum be a good experience on super tiny screens or a good experience on very popular iPhone screens?

I don't understand what you mean, we are not forced to choose between both, this pull request fixes all issues.

I didn’t look at the pull request. I didn’t realize it included JavaScript too. Sounds great! At least I have a CSS patch for my sites for now which seems to work. I’ll probably do some tiny screen tests later to see what all gets hidden.

I like the pop up... but, sometimes I wonder if there’s a better fully responsive way. Because it’s not fully responsive right? That’s why we’re having the screen size issues? And having to rely on JavaScript to position it well?

I’m sure there are really good reasons. But, I wonder why the post box on mobile isn’t just like the post box on desktop? Where it is responsive and slides up into view.

SychO9 · 2021-01-03T15:00:27Z

I like the pop up... but, sometimes I wonder if there’s a better fully responsive way. Because it’s not fully responsive right? That’s why we’re having the screen size issues? And having to rely on JavaScript to position it well?

That's what this pull request does, removes the need to position it with javascript, and fixes it to the screen instead. Currently the behavior is an absolute and a set position with JS, so that one can see the discussion behind while scrolling (but there is no need for that since you can minimize the composer anyway). The composer element itself is responsive because it adapts to a better view on smaller screens, it's just that because of the absolute position (changed to fixed in this PR) it creates the 2 linked to issues.

zerosonesfun · 2021-01-03T15:08:14Z

I like the pop up... but, sometimes I wonder if there’s a better fully responsive way. Because it’s not fully responsive right? That’s why we’re having the screen size issues? And having to rely on JavaScript to position it well?

That's what this pull request does, removes the need to position it with javascript, and fixes it to the screen instead. Currently the behavior is an absolute and a set position with JS, so that one can see the discussion behind while scrolling (but there is no need for that since you can minimize the composer anyway). The composer element itself is responsive because it adapts to a better view on smaller screens, it's just that because of the absolute position (changed to fixed in this PR) it creates the 2 linked to issues.

Ok, thanks for explaining! I didn’t get it at first. I’ll hold my breath and cross my figures that all of the devs agree with this pull request for beta 16. I would think it needs to be in beta 16 to ensure full testing before stable.

SychO9 · 2021-01-03T15:35:56Z

Well thank you for testing and looking into this, it's really appreciated!

Also if you can't test by making the JS changes (it's more tedious to do so I know) you can use this, I don't think (not sure) it works well when composer is minimized, but it at least allows you to test when the composer is open on mobile :) without having to touch the JS

@media (max-width: 767px) {
    .Composer:not(.minimized) {
        position: fixed;
        top: 0 !important;
    }
}

zerosonesfun · 2021-01-03T17:18:17Z

Well thank you for testing and looking into this, it's really appreciated!

Also if you can't test by making the JS changes (it's more tedious to do so I know) you can use this, I don't think (not sure) it works well when composer is minimized, but it at least allows you to test when the composer is open on mobile :) without having to touch the JS
@media (max-width: 767px) {
    .Composer:not(.minimized) {
        position: fixed;
        top: 0 !important;
    }
}

This works great! So, I guess the idea behind having it stuck to the top instead of stuck to the bottom has to do with previewing and tiny screens? My brain is like... hmm, having it at the top is fine. But, comment boxes are usually at the bottom of UIs, no? Regardless I’m going with this CSS now at my sites until there’s something permanent. I’m sure there are very good reasons for going with sticking to top versus bottom. And, I’m sure the JS change works too by the way. If it works for you, I don’t see why it wouldn’t work on iOS.

SychO9 · 2021-01-03T17:49:02Z

So, I guess the idea behind having it stuck to the top instead of stuck to the bottom has to do with previewing and tiny screens? My brain is like... hmm, having it at the top is fine. But, comment boxes are usually at the bottom of UIs, no?

having it at the bottom does look and behave better imo, but although I haven't tested, I am pretty sure if it were fixed to the bottom, smaller mobile screens would have a portion of the composer element hidden when the keyboard is open, because there wouldn't be enough space for both, so it just feels safer to fix it at the top.

askvortsov1 · 2021-01-05T06:47:12Z

I am pretty sure if it were fixed to the bottom, smaller mobile screens would have a portion of the composer element hidden when the keyboard is open, because there wouldn't be enough space for both, so it just feels safer to fix it at the top.

If there isn't enough room for both, wouldnt it not matter if the composer is at the top or bottom, since the lack of space implies that all space below/above the composer would be taken up, and the composer and keyboard would be the only 2 elements on the page?

SychO9 · 2021-01-05T19:11:44Z

I guess I'm just assuming it would be better if the keyboard hid a part of the composer's bottom, then if the keyboard pushed the composer up to hide it's top 🤷

askvortsov1

👍

This reverts commit 1e37cc0.

Fix composer header hidden by mobile browser

0e8024d

askvortsov1 requested changes Aug 29, 2020

View reviewed changes

askvortsov1 requested review from clarkwinkelmann and dsevillamartin August 31, 2020 17:13

stale bot added the stale Issues that have had over 90 days of inactivity label Dec 23, 2020

SychO9 removed the stale Issues that have had over 90 days of inactivity label Dec 23, 2020

askvortsov1 approved these changes Jan 5, 2021

View reviewed changes

tankerkiller125 approved these changes Jan 6, 2021

View reviewed changes

askvortsov1 merged commit 9b27b0d into flarum:master Jan 7, 2021

SychO9 deleted the sim/fix-composer-edge-mobile branch January 7, 2021 18:25

SychO9 mentioned this pull request Feb 19, 2021

Composer stuck when creating new post on mobile #2617

Closed

matteocontrini pushed a commit to fibraclick/flarum-framework that referenced this pull request Feb 19, 2021

Fix composer header hidden by mobile browser (flarum#2279)

1e37cc0

matteocontrini mentioned this pull request Feb 22, 2021

Composer is partially hidden on small mobile screens when keyboard is open #2628

Closed

matteocontrini added a commit to fibraclick/flarum-framework that referenced this pull request Feb 22, 2021

Revert "Fix composer header hidden by mobile browser (flarum#2279)"

c1e64a0

This reverts commit 1e37cc0.

matteocontrini mentioned this pull request Mar 2, 2021

Composer goes off screen on iOS when starting to type #2652

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix composer header hidden by mobile browser #2279

Fix composer header hidden by mobile browser #2279

SychO9 commented Aug 29, 2020

askvortsov1 Aug 29, 2020

SychO9 Aug 30, 2020

SychO9 Aug 30, 2020

askvortsov1 Sep 24, 2020

stale bot commented Dec 23, 2020

zerosonesfun commented Jan 3, 2021

matteocontrini commented Jan 3, 2021

SychO9 commented Jan 3, 2021 •

edited

Loading

zerosonesfun commented Jan 3, 2021

zerosonesfun commented Jan 3, 2021 •

edited

Loading

SychO9 commented Jan 3, 2021

zerosonesfun commented Jan 3, 2021 •

edited

Loading

SychO9 commented Jan 3, 2021

zerosonesfun commented Jan 3, 2021

SychO9 commented Jan 3, 2021

zerosonesfun commented Jan 3, 2021 •

edited

Loading

SychO9 commented Jan 3, 2021

askvortsov1 commented Jan 5, 2021

SychO9 commented Jan 5, 2021

askvortsov1 left a comment •

edited

Loading

Fix composer header hidden by mobile browser #2279

Fix composer header hidden by mobile browser #2279

Conversation

SychO9 commented Aug 29, 2020

askvortsov1 Aug 29, 2020

Choose a reason for hiding this comment

SychO9 Aug 30, 2020

Choose a reason for hiding this comment

SychO9 Aug 30, 2020

Choose a reason for hiding this comment

askvortsov1 Sep 24, 2020

Choose a reason for hiding this comment

stale bot commented Dec 23, 2020

zerosonesfun commented Jan 3, 2021

matteocontrini commented Jan 3, 2021

SychO9 commented Jan 3, 2021 • edited Loading

zerosonesfun commented Jan 3, 2021

zerosonesfun commented Jan 3, 2021 • edited Loading

SychO9 commented Jan 3, 2021

zerosonesfun commented Jan 3, 2021 • edited Loading

SychO9 commented Jan 3, 2021

zerosonesfun commented Jan 3, 2021

SychO9 commented Jan 3, 2021

zerosonesfun commented Jan 3, 2021 • edited Loading

SychO9 commented Jan 3, 2021

askvortsov1 commented Jan 5, 2021

SychO9 commented Jan 5, 2021

askvortsov1 left a comment • edited Loading

Choose a reason for hiding this comment

SychO9 commented Jan 3, 2021 •

edited

Loading

zerosonesfun commented Jan 3, 2021 •

edited

Loading

zerosonesfun commented Jan 3, 2021 •

edited

Loading

zerosonesfun commented Jan 3, 2021 •

edited

Loading

askvortsov1 left a comment •

edited

Loading