Adding header in theme messes up topic progress bar (when docked)

support
1

When we added a header in our theme, on mobile, the toggle where the #topic-progress-wrapper div position is toggled from fixed to absolute, the bottom property is set wrong (doesn’t include the header when calculating the body length).

A little inspection and a posting from @Steven from 2016 provided enough of a solution but I hate leaving CSS cruft like this in our theme.

Seems like wherever the bottom property is calculated dynamically should be able to figure out the correct body length (including header)?

I hate to file this as a bug if I’m just missing something…

1 Like
2

Can you provide code specifics, maybe @Johani can advise?

1 Like
3

I’ll try - still trying to figure it out as my time for this permits…

All this is inspected on mobile view only…

When reading any topic which is either short or when scrolled to the bottom, the block:

<div id="topic-progress-wrapper" class="ember-view" style="right: 1em;"> 
...
</div>

is changed to

<div id="topic-progress-wrapper" class="docked ember-view" style="right: 1em; bottom: 82.4988px;">
...
</div>

Obviously, the bottom: property is calculated and the .docked class changes the position property from fixed to absolute. The bottom property seems to be calculated in (in the ember component discourse/components/topic-progress pretty-printed from minified in inspector):

        _dock: function() {
            var e = this.$();
            if (e && 0 !== e.length) {
                var t = window.pageYOffset || $("html").scrollTop()
                  , n = this.site.mobileView ? 0 : $("#topic-progress").height()
                  , i = $("#topic-bottom").offset().top + n
                  , o = $(window).height()
                  , s = $("#reply-control").height() || 0
                  , a = t >= i - o + s
                  , r = $("#main").height() - i;
                s > 0 ? e.css("bottom", a ? r : s) : e.css("bottom", a ? r : ""),
                this.set("docked", a);
                var l = $("#reply-control .reply-area");
                l && l.length > 0 ? e.css("right", l.offset().left + "px") : e.css("right", "1em")
            }
        },

Since this calculates the bottom property based on the window height of the #main selector, it does not include our header and, hence, the bottom property locates the box offset from the desired location.

Could bottom be calculated from the body.docked selector in this case instead?

(that’s all I have time for today…)

3 Likes
4

So, the code in question is:

  _dock() {
    const $wrapper = this.$();
    if (!$wrapper || $wrapper.length === 0) return;

    const offset = window.pageYOffset || $("html").scrollTop();
    const progressHeight = this.site.mobileView
      ? 0
      : $("#topic-progress").height();
    const maximumOffset = $("#topic-bottom").offset().top + progressHeight;
    const windowHeight = $(window).height();
    const composerHeight = $("#reply-control").height() || 0;
    const isDocked = offset >= maximumOffset - windowHeight + composerHeight;
    const bottom = $("#main").height() - maximumOffset;

    if (composerHeight > 0) {
      $wrapper.css("bottom", isDocked ? bottom : composerHeight);
    } else {
      $wrapper.css("bottom", isDocked ? bottom : "");
    }

    this.set("docked", isDocked);

    const $replyArea = $("#reply-control .reply-area");
    if ($replyArea && $replyArea.length > 0) {
      $wrapper.css("right", `${$replyArea.offset().left}px`);
    } else {
      $wrapper.css("right", "1em");
    }
  },

I have proven to myself that, in my case, changing

   const bottom = $("#main").height() - maximumOffset;

to

   const bottom = $("body").height() - maximumOffset;

works perfectly for our site.

I can imagine that you may want to not presume the body element selector would work in all cases so a perhaps more elegant solution would be to wrap the theme (brand?) header in a div that could be selected such that you could replace the bottom calculation with something like:

    const bottom = ((($("#brand_header").height() + $("#main").height()) - maximumOffset;

Suggestions? Where can we go from here?

1 Like
5

Is your instance public? I tried to follow your code description, but this would be easier to track down if I could inspect your site. (For example, I added this component to a test site and couldn’t find any issues with the progress bar on mobile.)

4 Likes
6

It’s not public but I will PM you a login on our test site and pull all our special code out of the theme…

7

Thanks @Lew_Grothe, I do see the issue now. There are two lines of code that cause this when using a theme header, first:

const maximumOffset = $("#topic-bottom").offset().top + progressHeight;

offset.top here gets the element’s position from the top of the document. Later in the code, the position of the progress bar is calculated based on the height of #main minus said offset:

const bottom = $("#main").height() - maximumOffset;

But when adding an element to the body before #main, the offset will be misaligned with #main. I will prepare a PR tomorrow and propose to target the body element, as you suggest, it makes sense to me.

6 Likes
8

Any guess as to what release this might be in?

9

Whenever this PR is merged :wink:

https://github.com/discourse/discourse/pull/6425

EDIT: this PR is now merged

7 Likes
10

This topic was automatically closed 30 days after the last reply. New replies are no longer allowed.