Add setting to toggle PR icons in Graph #2450
Conversation
axosoft-ramint
added
the
area-graph
package.json
Outdated
| @@ -2250,6 +2250,13 @@ | |||
| "scope": "window", | |||
| "order": 26 | |||
| }, | |||
| "gitlens.graph.showPullRequests": { | |||
There was a problem hiding this comment.
Lets call it gitlens.graph.pullRequests.enabled to align with the other PR related options
There was a problem hiding this comment.
Done in commit: Rename setting
src/plus/webviews/graph/protocol.ts
Outdated
| export const GraphRefMetadataTypes = { | ||
| Upstream: 'upstream', | ||
| PullRequest: 'pullRequests', | ||
| }; |
There was a problem hiding this comment.
A note on this piece: I originally had it pulling these strings directly from the graph library. However, this caused the GitLens extension to fail to load entirely when debugging, producing a document is not defined error:
I'm guessing it's because I was attempting to pull constants out of a typedef file, but not sure exactly why it failed (it built successfully).
There was a problem hiding this comment.
Yeah, can't use consts as values from d.ts files. d.ts are just types/shapes and are not available at runtime.
That's why https://github.dev/gitkraken/vscode-gitlens/blob/52d8b0ad6b5a0d33781410dae49af11c5fba1dc8/src/%40types/vscode.git.enums.ts#L1 exists, I had to separate out certain consts from the vscode.git.d.ts file so that they would get compiled in
There was a problem hiding this comment.
Thanks! We'll definitely want an enums export in the Graph in general then - I'll add that in as a separate task.
There was a problem hiding this comment.
I would also suggest changing this to:
export enum GraphRefMetadataTypes {
Upstream = 'upstream',
PullRequest = 'pullRequests',
}
export const supportedRefMetadataTypes: GraphRefMetadataType[] = Object.values(GraphRefMetadataTypes);To avoid having to call Object.values each time when checking the metadata, and that will also make sure that enum stays in sync with the types, since the supportedRefMetadataTypes will fail if an enum/type value don't change together
There was a problem hiding this comment.
Done in commit: Use enum and values array
| >Show associated pull requests on remote branches</label | ||
| > | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Add
<p class="setting__hint hidden" data-visibility="currentLine.pullRequests.enabled">
<i class="icon icon__info"></i>Requires a connection to a supported remote service (e.g. GitHub)
</p>There was a problem hiding this comment.
Thanks. Should I add that for the upstream indicator setting as well?
There was a problem hiding this comment.
Done in commit: Add hints for remote service connection
There was a problem hiding this comment.
Upstream, doesn't need an integration -- just the PRs
Upstream we get completely from Git, but the PRs require a rich integration to pull the details from GitHub/GitLab
There was a problem hiding this comment.
Got it, thanks! Fixed in commit: Remove remote service hint from upstream setting
| @@ -170,6 +170,20 @@ <h2> | |||
| </div> | |||
| </div> | |||
|
|
|||
| <div class="setting"> | |||
There was a problem hiding this comment.
Can do it outside this PR, but we should look at the UX/UI of the graph settings and group some of them up to make them easier for users to understand
There was a problem hiding this comment.
Wrote up a task for it, thanks!
|
@eamodio The dependency on the graph side has been merged. Bumped the graph version in this PR to match it. |
We had a setting to enable/disable upstream status for local branches in the graph. This adds a setting to toggle the other metadata type: Pull Requests.
This also combines all the active metadata from settings into a single array to send to the graph. This requires a graph library update after the breaking change lands: https://github.com/gitkraken/GitKrakenComponents/pull/189
Once that merges, the dependency will be updated in this PR and it can leave draft state.