fix(extension): resolve sticky-offset overlap and apply missed PR-feedback fixes#400
Conversation
…dback fixes The previous PR (#395) intended to address four Copilot review threads but a botched amend dropped the fixes from the working tree. This commit re-applies them, plus fixes a separate bug where our nav row overlaps the file-tree sticky pane on the Files tab. 1. Negative caching in fetchCommentBodies: a 404/5xx from edit_form is now remembered for 60s so a deleted comment or a transient failure no longer causes a refetch on every tryInject tick. 2. Stale UI removal in renderMergifyContext: the early-return paths for "no comment bodies" and "panel is null" now explicitly remove any existing #mergify-context and #mergify-stack-nav. Previously the panel could linger after the markers disappeared. 3. In-flight dedup for status fetches: gatherPrStatuses now reuses a single Promise per (org/repo/num/head_sha) so multiple ticks during React reconciliation don't fire duplicate fetchPrStatus requests. 4. Toolbar reset on null nav path: injectStackNav now resets flex-wrap and removes the padding-bottom override unconditionally, not gated on whether our nav node still exists. React can drop our node while leaving the inline styles behind. 5. Sticky-offset overlap: GitHub's diff-view file-tree pane uses position: sticky; top: 60px hardcoded to the toolbar's original height. Our nav grows the toolbar to ~93px, which would otherwise hide the pane behind our nav. injectStackNav now writes a small <style> rule overriding the pane's top to the toolbar's actual current height (with !important to outweigh the class rule). The rule is removed when our nav is removed. Five new tests cover each fix. Change-Id: Ie79f4d58085da30fc97aa50b60c0e0d4468b4e16
Member
Author
|
This pull request is part of a Mergify stack:
|
Contributor
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🔴 ⛓️ Depends-On RequirementsWaiting for:
This rule is failing.Requirement based on the presence of
🔴 Required ReviewsWaiting for:
This rule is failing.
🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 🔎 ReviewsWonderful, this rule succeeded.
🟢 📕 PR descriptionWonderful, this rule succeeded.
|
jd
deleted the
devs/jd/worktree-leverage-json/resolve-sticky-offset-overlap-apply-missed-pr--e79f4d58
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The previous PR (#395) intended to address four Copilot review threads
but a botched amend dropped the fixes from the working tree. This
commit re-applies them, plus fixes a separate bug where our nav row
overlaps the file-tree sticky pane on the Files tab.
Negative caching in fetchCommentBodies: a 404/5xx from edit_form
is now remembered for 60s so a deleted comment or a transient
failure no longer causes a refetch on every tryInject tick.
Stale UI removal in renderMergifyContext: the early-return paths
for "no comment bodies" and "panel is null" now explicitly remove
any existing #mergify-context and #mergify-stack-nav. Previously
the panel could linger after the markers disappeared.
In-flight dedup for status fetches: gatherPrStatuses now reuses a
single Promise per (org/repo/num/head_sha) so multiple ticks
during React reconciliation don't fire duplicate fetchPrStatus
requests.
Toolbar reset on null nav path: injectStackNav now resets
flex-wrap and removes the padding-bottom override unconditionally,
not gated on whether our nav node still exists. React can drop our
node while leaving the inline styles behind.
Sticky-offset overlap: GitHub's diff-view file-tree pane uses
<style> rule overriding the pane's top to the toolbar's actual current height (with !important to outweigh the class rule). The rule is removed when our nav is removed.position: sticky; top: 60px hardcoded to the toolbar's original
height. Our nav grows the toolbar to ~93px, which would otherwise
hide the pane behind our nav. injectStackNav now writes a small
Five new tests cover each fix.
Depends-On: #398