Skip to content

App context menu opens to WIP row on graph#2458

Merged
ramin-t merged 3 commits intomainfrom
feature/graph-wip-context-menu
Jan 26, 2023
Merged

App context menu opens to WIP row on graph#2458
ramin-t merged 3 commits intomainfrom
feature/graph-wip-context-menu

Conversation

@ramin-t
Copy link
Contributor

@ramin-t ramin-t commented Jan 19, 2023

Closes: #2419

Adds three context menu options to work-in-progress row on graph to stash changes, open in commit details, and open SCM:

WIP context menu

@ramin-t ramin-t requested a review from eamodio January 19, 2023 21:05
@ramin-t ramin-t added area-graph Issues or features related to the Commit Graph feature labels Jan 19, 2023
Copy link
Member

@eamodio eamodio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good overall, just some comments inline.

@eamodio
Copy link
Member

eamodio commented Jan 20, 2023

One question from a UX point of view -- should Stash All Changes come after the "opens"? Generally, the way you have it follows our patterns, but I wonder in this case if the 2 opens are the much more likely actions. /cc @d13 @justinrobots

@ramin-t
Copy link
Contributor Author

ramin-t commented Jan 20, 2023

One question from a UX point of view -- should Stash All Changes come after the "opens"? Generally, the way you have it follows our patterns, but I wonder in this case if the 2 opens are the much more likely actions. /cc @d13 @justinrobots

Good point and would like feedback from @d13 and @justinrobots

It's a tough one to answer, because sometimes I'm committing my WIP stuff more frequently (in which case I would prefer Open Source Control on top) and sometimes I'm stashing more frequently (in which case I would prefer Stash All Changes on top).

Overall I find that I stash more often, as usually I'm writing stuff on main and end up wanting it on its own branch, or I need to switch context for a bit to test a PR before going back to my changes, so my vote would be to have Stash All Changes on top.

@ramin-t
Copy link
Contributor Author

ramin-t commented Jan 26, 2023

@eamodio Haven't heard any additional feedback on this (regarding the UX). Should we go ahead with the current ordering of the menu items and then rearrange them if needed once we get feedback after this lands?

@ramin-t ramin-t requested a review from eamodio January 26, 2023 18:08
Copy link
Member

@eamodio eamodio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great -- yeah, let's get this in, we can always change the order later.

@ramin-t ramin-t merged commit 805c49b into main Jan 26, 2023
@ramin-t ramin-t deleted the feature/graph-wip-context-menu branch January 26, 2023 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-graph Issues or features related to the Commit Graph

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a context menu to the WIP row in the Graph

2 participants