gordonwoodhull
commented
5 years ago I think it's fine to go straight to Apply Changes, since there is still no action taken at that point. There isn't anything interesting the user could do at the Merge Changes dialog, and they can still cancel by clicking the button or pressing esc.
I think that people may use the Merge Notebook feature as a way to quickly bring up a diff, so all the better to go straight to it.
(The "current merge source" for the notebook should still be updated when the button is clicked.)
Speaking of accidental clicks, the Merge button is very close to the Remove button. I guess this has to do with the shape of the icon; it wasn't an issue when hide/show notebook was in that spot. It looks like the framework for adding buttons has the ability to customize the style, so could you please add a tiny bit of padding there?
Finally, I think "merge" is somewhat ambiguous - could you please change the popup to read "merge here"? (Or something... "merge into this notebook" would be too long.)
Overall this looks great and will be very useful. I'll wait on these tweaks to merge it.
Thanks @JABedford!
jameesy
Fixes #2597 & Fixes #2612
The merge button is displayed in the tree, and when clicked on will open a new merge dialogue. The ID of the notebook that the user clicked on will automatically be present in the ID input box allowing the user to continue.
I wasn't sure whether the intention of this feature was to take the user straight through to the "Stage Changes" dialogue, but actually I think directing to the initial merge screen is perhaps a more graceful solution as it gives the user a "confirmation" screen, rather than staging the changes, incase the Merge button was clicked on accidentally.
I added the fix to the folder renaming issue just because it was a quick fix in the same file I had been working in.