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

Fixes 2 more tasks in #464 - regarding viewport correct sizing on client #467

Merged
merged 1 commit into from
Nov 11, 2023

Conversation

o0101
Copy link
Collaborator

@o0101 o0101 commented Nov 11, 2023

The recent updates address an issue caused by excessive optimization in our approach. Previously, we aimed to minimize how often we reset the viewport. However, this strategy led to a bug affecting non-active tabs. Here's the scenario:

  • A non-active tab is open at a larger size.
  • A client with a smaller screen connects.
  • Our system, which only remembered the viewport size for the active tab, would update the viewport for the active tab.
  • When switching back to the larger, non-active tab, the system failed to recognize the need for a viewport adjustment. This happened because it was comparing against the updated smaller viewport size and mistakenly concluding no change was needed.

The root cause? We only had a single, browser-wide memory for viewport size, rather than individual memory for each target (tab).

The solution? We've now implemented a per-target (per-tab) viewport memory system. This enhancement enables the correct adjustment of viewport sizes for each tab individually, effectively resolving the bug.

…ent changes

The new fixes attempt to fix an effect of over optimization. We'd previously been trying to limit the amount of time we reset the viewport. That had caused a bug where we missed when a non-active tab was in an old larger size prior to a smaller client connect, but as we only had a single 'per browser' viewport memory, and we'd already update the viewport of the active tab, when switching back to this larger tab, after the browser viewport had already been updated, our test showed there was no change to be made. This is because we didn't have a per target viewport memory. Now we do. This fixes the bug.
@o0101 o0101 merged commit 8e7e398 into boss Nov 11, 2023
3 checks passed
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.

None yet

1 participant