Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Conversation

@smashwilson
Copy link
Contributor

@smashwilson smashwilson commented Jan 31, 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

Restores the behavior of following the WorkdirContext associated with the active pane item. Introduces the concept of a "locking" a WorkdirContext, so that one chosen explicitly from a context tile can be preserved even as you navigate elsewhere. Adds a padlock icon to the context tile to control toggling the lock.

Hat tip to @jarvelov for the locking idea! Solves a tricky UX problem in an elegant way.

Remaining work

  • Introduce context locking in GitHubPackage
  • Restore active pane following in GitHubPackage
  • Drill context lock state and context lock manipulation callbacks into the git and GitHub tab react trees
  • Implement context lock toggling action in the appropriate controllers
  • Add the context lock toggle control to the context tile views
  • Find an SVG editor and edit the lock octicon to make an unlocked svg icon.
  • Actually run the code and fix the inevitable bugs 😄
  • Tests passing and full change coverage

Screenshot/Gif

context lock

Alternate Designs

See #2335 for an extended discussion about this.

Benefits

This will allow us to preserve the benefits of having visibility of, and explicit control over, the current WorkdirContext, while reintroducing the convenience of the context following active workspace items.

Possible Drawbacks

It will still be possible for users to get lost if they do something like open a file outside of their working repo for a quick configuration edit; see #1595 for some of this confusion. With this change, they will be able to lock their context to prevent this from happening, but if they aren't already working across multiple projects

Applicable Issues

Fixes #2335.

Documentation

N/A

Release Notes

  • Git repository context may be "locked" to manage manually or "unlocked" to follow the active pane item.

User Experience Research (Optional)

N/A

Introduce a context lock to prevent this from happening when we don't 
want it to.
@codecov
Copy link

codecov bot commented Jan 31, 2020

Codecov Report

Merging #2399 into master will decrease coverage by 0.07%.
The diff coverage is 97.58%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2399      +/-   ##
==========================================
- Coverage   93.43%   93.35%   -0.08%     
==========================================
  Files         235      235              
  Lines       13066    13147      +81     
  Branches     1859     1885      +26     
==========================================
+ Hits        12208    12274      +66     
- Misses        858      873      +15
Impacted Files Coverage Δ
lib/controllers/github-tab-controller.js 100% <ø> (ø) ⬆️
lib/views/git-tab-view.js 86.36% <ø> (-0.16%) ⬇️
lib/controllers/git-tab-controller.js 80.67% <ø> (ø) ⬆️
lib/views/github-tab-view.js 100% <ø> (+4.54%) ⬆️
lib/controllers/root-controller.js 83.78% <ø> (ø) ⬆️
lib/atom/octicon.js 100% <100%> (ø) ⬆️
lib/controllers/github-tab-header-controller.js 100% <100%> (ø) ⬆️
lib/controllers/git-tab-header-controller.js 100% <100%> (ø) ⬆️
lib/views/github-tab-header-view.js 100% <100%> (+7.69%) ⬆️
lib/containers/github-tab-header-container.js 100% <100%> (ø) ⬆️
... and 5 more

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 6d10136...36d001e. Read the comment docs.

@wadethestealth
Copy link
Contributor

Looking pretty good so far! 👍

@smashwilson
Copy link
Contributor Author

Thanks! ✨

Just need to get the "unlock" icon looking right, then write the hover copy for the lock button states, I think.

@smashwilson smashwilson marked this pull request as ready for review February 5, 2020 20:19
@smashwilson smashwilson merged commit 5246a4a into master Feb 5, 2020
@smashwilson smashwilson deleted the lock-context branch February 5, 2020 20:21
@aviatesk
Copy link

aviatesk commented Feb 5, 2020

Thanks for your work, @smashwilson ! Looking forward to giving a try on this !

@jarvelov
Copy link

jarvelov commented Feb 6, 2020

Awesome work @smashwilson! 🎉

@mordash
Copy link

mordash commented Mar 12, 2020

Awesome ! thanks

@pouruthi
Copy link

pouruthi commented May 2, 2020

I

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Git context management

7 participants