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

Populate StubItem panes that are deserialized after package initialization #2415

Merged
merged 3 commits into from Mar 7, 2020

Conversation

smashwilson
Copy link
Member

@smashwilson smashwilson commented Mar 6, 2020

Please be sure to read the contributor's guide to the GitHub package before submitting any pull requests.

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • Suggestion: You can use checklists to keep track of progress for the sections on metrics, tests, documentation, and user research.

Description of the Change

We use StubItems to temporarily deserialize React-managed pane items, like the git or GitHub tabs, before the React tree is initialized and actually renders DOM elements within them to make them useful. The <PaneItem> component handles the logic of finding and "hydrating" any stubs that were deserialized from persisted workspace state on Atom launch. However, it's possible that StubItems will be deserialized directly into the workspace after the package is initialized, and <PaneItem> will never find them!

As a result, a user who opens a project after Atom has been launched - for example, from the File -> Reopen Project menu - will be stuck with blank stub panels until they close and re-open the items.

To fix this, let's watch newly added pane items on the workspace, detect when they're stubs that we don't already own, and hydrate them anyway.

Screenshot/Gif

Before:

blank-stub

After:

(TBD)

Alternate Designs

I'd love to revisit the way that StubItems work... I think we could simplify that way that they're created and managed to have a single "Item" class that's used consistently regardless of whether the item was opened through the <PaneItem>-registered opener function or deserialized from project state. That's a heavier change though and I feel like it'd risk introducing more regressions than it would fix, so I'm preferring the band-aid fix for now.

Benefits

At least one class of scenarios that would result in orphaned, blank StubItems will be patched.

Possible Drawbacks

We'll need to do more work on each pane item opening. This could result in performance impacts when large numbers of items are open, although I'll try to avoid it.

Applicable Issues

Fixes #2414.

Metrics

N/A

Tests

Following the verification steps in the linked issue, and adding unit tests.

Documentation

N/A

Release Notes

  • Fixed a bug that would result in the Git and GitHub tabs being blank when deserialized from project state.

User Experience Research (Optional)

N/A

@codecov
Copy link

codecov bot commented Mar 6, 2020

Codecov Report

Merging #2415 into master will decrease coverage by 0.00%.
The diff coverage is 93.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2415      +/-   ##
==========================================
- Coverage   93.42%   93.41%   -0.01%     
==========================================
  Files         235      235              
  Lines       13147    13163      +16     
  Branches     1885     1889       +4     
==========================================
+ Hits        12282    12296      +14     
- Misses        865      867       +2     
Impacted Files Coverage Δ
lib/atom/gutter.js 90.47% <0.00%> (-2.39%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f9c0af...01791d1. Read the comment docs.

@smashwilson smashwilson marked this pull request as ready for review Mar 7, 2020
@smashwilson smashwilson merged commit aaf6411 into master Mar 7, 2020
2 checks passed
@smashwilson smashwilson deleted the blank-panels branch Mar 7, 2020
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.

GIT and GITUB Panel blank after start ATOM and open Project
1 participant