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

Show a message when opening a file change redirects to GitHub #442

Closed
miguelsolorio opened this issue Sep 12, 2018 · 5 comments
Closed

Show a message when opening a file change redirects to GitHub #442

miguelsolorio opened this issue Sep 12, 2018 · 5 comments
Labels
feature-request Request for new features or functionality ux

Comments

@miguelsolorio
Copy link
Contributor

miguelsolorio commented Sep 12, 2018

If you open a file from a PR that contains an unsupported file type, it automatically opens your browser and navigates to the GitHub website and shows the preview. This is quite jarring and we should provide a better experience by showing a message and giving the user an option to preview on the web.

Repro steps

  1. Navigate to PR that has changes to an svg/ttf/unsupported file type (i.e. Add .glsl icon jesseweed/seti-ui#504)
  2. Click on file in the GitHub PR Viewlet
  3. You are re-directed to the GitHub that shows the file

Expected

  1. Show a message that explains that file is not supported with a link to preview on the web
@RMacfarlane RMacfarlane added ux feature-request Request for new features or functionality labels Sep 12, 2018
@RMacfarlane RMacfarlane changed the title Opening an unsupported file type should show a message Show a message when opening a file change redirects to GitHub Sep 14, 2018
@RMacfarlane
Copy link
Contributor

RMacfarlane commented Sep 14, 2018

Updated the title, since there are other situations where we open the file on GitHub that we should explain, for example when it's too large. The GitHub API doesn't actually return a reason for why the patch isn't included, so we can either try to infer it or have a more generic message

@thisissami
Copy link

Yeah I 100% agree on this. It definitely breaks my flow when doing a peer review in VSCode to unexpectedly have my browser open when clicking on the next file in the directory. In my case it was purely a renamed file - I see no reason why it can't just be shown in VSCode, or at the very least - a message saying something along the lines of This has solely been renamed and is accordingly not available here. Do you wish to view it on GitHub?

@eamodio
Copy link
Contributor

eamodio commented Oct 3, 2018

Unsupported types are one thing, but for a file that is too large (or doesn't have the patch included), can't you just get the whole contents of the file at that version to provide the diff?

@Maskime
Copy link

Maskime commented Oct 18, 2019

Hi,

I'm commenting on this because I don't where to put this otherwise.
I understand that you're facing a limitation from github that won't let you fetch all the file content but here are some comments :

  1. From my experince : The message that we get from the extension is misleading : "This file is either too large, of an supported type, or has only be renamed...". It took me some digging to see that it was not that my file that was too large, but the pull request that was in fact too large.
  2. Are people from github aware that you're facing this situation ? This problem is kind of a show stopper for me, as I'm using VS Code to go through the massive pull request that would just kill my browser when opened directly on github.

Anyway thanks for the hard work, really hope that you'll find some actual solution to this diff size problem.

Best regards

@RMacfarlane
Copy link
Contributor

@Maskime Thanks for your feedback! This sounds similar to #1160.

I've previously reached out to GitHub to try to clarify what the exact circumstances are for patches not being returned and am still waiting for a response. This is definitely a pain point! With some refactoring we may be able to handle this by just making additional network calls when trying to open the diff to resolve the whole file contents of both sides instead of working with a patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality ux
Projects
None yet
Development

No branches or pull requests

5 participants