Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Scrolling in Minecraft HOC Tutorial View #10235

Merged
merged 6 commits into from
Oct 17, 2024

Conversation

thsparks
Copy link
Contributor

@thsparks thsparks commented Oct 16, 2024

Fixes https://github.com/microsoft/pxt-minecraft/issues/2579

Height Calculation

We were trying to compute the tutorial pane's height directly to the scroll height of the content plus a hard-coded buffer, and I think the fixed buffer eventually got out of date when the styles changed.

Rather than simply updated the buffer size to a different fixed value, I set the height to auto which just dynamically resizes to fit content (Googling tells me it's well supported across different browsers, so I think it's safe).

There was also an issue where the clientHeight / scrollHeight comparison was slightly off when determining if there was overflow because the clientHeight value contains padding, which eats into the actual "user visible" area for the child content. I've added a function to calculate the height without the padding to account for this

Show More/Less

While testing, I noticed the "Show More"/"Less" toggle appeared and disappeared somewhat randomly (even without my changes). The check we were doing for this was always based on the current size of the element, so I believe the decision on whether to show it was being affected by whether the element was actually expanded (i.e. "Show More") or not. This led to inconsistent-feeling behavior.

I reworked this so it's now based on the initial size as a "default" and not just the latest current size. I also moved it to the left when the toolbox is hidden, since it was just empty space before and I think it's better than overlapping the undo/redo buttons.

See It, Try It

Upload Target

Screenshots
image
image
image

image
image

…cific numbers plus a hardcoded threshold. Also change how we determine if we need to show the more/less toggle, so it's now based on the initial size as a "default" and not just the latest current size, which leads to inconistent behavior.
…mpty space, vs overlapping the undo/redo buttons).
@thsparks thsparks requested a review from a team October 16, 2024 23:12
Copy link
Contributor

@srietkerk srietkerk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@thsparks thsparks merged commit a7e5cde into master Oct 17, 2024
6 checks passed
@thsparks thsparks deleted the thsparks/mc/fix_tutorial_scroll branch October 17, 2024 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants