weixin_39788131
weixin_39788131
2021-01-12 17:08

Fix composer header hidden by mobile browser

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.

css
 (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: https://github.com/flarum/core/commit/e6e2cdd3e98be6268f4dd7fb370d366f565ae72e

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).~

该提问来源于开源项目:flarum/core

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享
  • 邀请回答

15条回答

  • weixin_39788131 weixin_39788131 3月前

    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

    less
     (max-width: 767px) {
        .Composer:not(.minimized) {
            position: fixed;
            top: 0 !important;
        }
    }
    
    点赞 评论 复制链接分享
  • weixin_39563132 weixin_39563132 3月前

    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

    css-less
     (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.

    点赞 评论 复制链接分享
  • weixin_39788131 weixin_39788131 3月前

    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.

    点赞 评论 复制链接分享
  • weixin_39633113 weixin_39633113 3月前

    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?

    点赞 评论 复制链接分享
  • weixin_39788131 weixin_39788131 3月前

    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 :shrug:

    点赞 评论 复制链接分享
  • weixin_39563132 weixin_39563132 3月前

    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.

    ~~~

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

    }
    

    } ~~~

    Looks like this:

    点赞 评论 复制链接分享
  • weixin_39576270 weixin_39576270 3月前

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

    点赞 评论 复制链接分享
  • weixin_39788131 weixin_39788131 3月前

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

    点赞 评论 复制链接分享
  • weixin_39563132 weixin_39563132 3月前

    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.

    点赞 评论 复制链接分享
  • weixin_39563132 weixin_39563132 3月前

    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.

    点赞 评论 复制链接分享
  • weixin_39788131 weixin_39788131 3月前

    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.

    点赞 评论 复制链接分享
  • weixin_39563132 weixin_39563132 3月前

    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.

    点赞 评论 复制链接分享
  • weixin_39788131 weixin_39788131 3月前

    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.

    点赞 评论 复制链接分享
  • weixin_39563132 weixin_39563132 3月前

    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.

    点赞 评论 复制链接分享
  • weixin_39875842 weixin_39875842 3月前

    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!

    点赞 评论 复制链接分享