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

Use preview tab for diff file views #1184

Merged
merged 9 commits into from Dec 8, 2022
Merged

Conversation

sheezaaziz
Copy link
Contributor

@sheezaaziz sheezaaziz commented Nov 15, 2022

Fixes #620

@github-actions
Copy link

Binder 👈 Launch a binder notebook on branch sheezaaziz/jupyterlab-git/620

@fcollonval fcollonval changed the title 620 Use preview tab for diff file views Nov 28, 2022
Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks @sheezaaziz I have a few suggestions.

src/components/CommitDiff.tsx Outdated Show resolved Hide resolved
src/commandsAndMenu.tsx Outdated Show resolved Hide resolved
src/commandsAndMenu.tsx Outdated Show resolved Hide resolved
src/commandsAndMenu.tsx Outdated Show resolved Hide resolved
src/components/diff/PreviewMainAreaWidget.ts Outdated Show resolved Hide resolved
src/commandsAndMenu.tsx Outdated Show resolved Hide resolved
Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thank @sheezaaziz

Two small things and we are good to go.

style/diff-nb.css Outdated Show resolved Hide resolved
src/components/diff/PreviewMainAreaWidget.ts Outdated Show resolved Hide resolved
Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks a lot @sheezaaziz

I tested and look again at the code. This is great. I rerun the tests failing hoping it is a random error.

@fcollonval fcollonval merged commit 55bb992 into jupyterlab:master Dec 8, 2022
@fcollonval
Copy link
Member

@all-contributors please add @sheezaaziz for code

@allcontributors
Copy link
Contributor

@fcollonval

I've put up a pull request to add @sheezaaziz! 🎉

@krassowski
Copy link
Member

krassowski commented Dec 24, 2022

Great work here, it is a nice feature to have!

  • Could we add a setting to disable this behaviour? It seems that add a setting for only using a single tab for diffs #620 proposed adding this as an opt-in setting.
  • When I move the diff to another spit panel (say to the right):
    • the italic style in tab bar disappears (probably the class does not stick after reattaching in another panel)
    • if I open another diff the old diff in right panel gets closed and reopened in the left panel. I find this annoying, I guess it was not intended.
  • I had no idea what italic style means before looking at the docs; it would be helpful if there was an explanation in the title, or maybe even a pin icon; a pin icon could be implemented as unicode prefix (📌 or 📍or 🖈) added to title

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add a setting for only using a single tab for diffs
3 participants