Refactor rich blob viewers and add Code|Rendered switch

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/24287

EE port: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1720

After https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10563, and while waiting for the build for https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1556 to succeed, I couldn't help myself.

We now have:

• a totally refactored backend for BlobViewers,
• a Source|Rendered switch that shows "Rendered" by default, except when the URL anchor starts with #L, which means line number anchors now work to link to specific lines in Markdown source!
• a file size limit of 10MB on client-side rendered files, so we don't force users to download huge files,
• and a .blob-viewer[data-type=simple] that rich JS viewers can fall back to when the rich rendering fails for whatever reason. They should hide() their own blob viewer (.blob-viewer[data-type=rich]) and the switch buttons (.js-blob-viewer-switcher), and show() that simple fallback viewer, which will be the regular Download or Text view.

To do:

• Split up in a sensible way
• Specs

The switch can be seen in action with SVG, but also supported for Markdown and IPython Notebooks, as well as other supported Markup formats:

/cc @rspeicher @jschatz1 @rymai @grzesiek

Cool, nice work @DouweM!

added 1 commit

• 487a2ad0 - WIP: Refactor rich blob viewers and add Code|Rendered switch

Compare with previous version

BlobViewer wishlist:

• GeoJSON (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5126)
• CSV (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7325)
• ZIP (using https://github.com/gildas-lormeau/zip.js or https://github.com/Stuk/jszip)

/cc @jschatz1

Edit: Looks like @dimitrieh already has a few dozen more at https://gitlab.com/gitlab-org/gitlab-ce/issues/21272#note_18635532 :)

mentioned in issue #21272 (closed)

This is neato @DouweM.

@DouweM but your switch would only work if it isn't binary right?

Or did you want to show a hex viewer.

@jschatz1 Yeah, the switch is only displayed for text based formats. A hex viewer would be interesting :D But let's not go there for now.

By the way, I think the JS viewers should have a max file size set on them, so that we don't force the user to download a huge file from the repo or LFS. We could also require the user to press some button to load the viewer if the file size is over some amount... Would 10MB be a good limit, or do we expect things like STL and Sketch files to be larger than that?

added 1 commit

• 94499532 - WIP

Compare with previous version

added 1 commit

• feec21c8 - WIP: Refactor rich blob viewers and add Code|Rendered switch

Compare with previous version

added 1 commit

• 1ec42c7b - WIP: Refactor rich blob viewers and add Code|Rendered switch

Compare with previous version

added 1 commit

• cb0d49cd - WIP: Refactor rich blob viewers and add Code|Rendered switch

Compare with previous version

mentioned in merge request !10563 (merged)

By the way, I think the JS viewers should have a max file size set on them, so that we don't force the user to download a huge file from the repo or LFS. We could also require the user to press some button to load the viewer if the file size is over some amount... Would 10MB be a good limit, or do we expect things like STL and Sketch files to be larger than that?

Yeah I agree, was something I was thinking about after the STL viewer was merged in. I'll work on that outside of this

The STL files might be larger than that (not sure about Sketch) but then I guess giving the user an option to view the file instead of being forced to download the file is better, especially if on a mobile device.

@DouweM At the moment it just hides & shows a div, which for large files just destroys the performance. Is there anyway we can do this through a link? ie. click code link & it has ?raw=1 at the end of the URL which shows the code instead?

added 1 commit

• 4ef228db - Improved toggle buttons

Compare with previous version

Yeah I agree, was something I was thinking about after the STL viewer was merged in. I'll work on that outside of this

@iamphill I already implemented the 10MB limit here :) Since this is likely to go into 9.2, there's no need for you to do any separate effort before this goes in.

The STL files might be larger than that (not sure about Sketch) but then I guess giving the user an option to view the file instead of being forced to download the file is better, especially if on a mobile device.

True, we can allow an override of the max file size limit. Let me look into that.

@DouweM At the moment it just hides & shows a div, which for large files just destroys the performance. Is there anyway we can do this through a link? ie. click code link & it has ?raw=1 at the end of the URL which shows the code instead?

@iamphill How do you mean destroys the performance? That's totally something we can do, but not requiring a reload is nice, and has this benefit:

[We now have] a .blob-viewer[data-type=simple] that rich JS viewers can fall back to when the rich rendering fails for whatever reason. They should hide() their own blob viewer (.blob-viewer[data-type=rich]) and the switch buttons (.js-blob-viewer-switcher), and show() that simple fallback viewer, which will be the regular Download or Text view.

How do you mean destroys the performance? That's totally something we can do, but not requiring a reload is nice, and has this benefit:

Yeah I really like that benefit as well. But on a IPython notebook file I have locally, it has ~5000 lines, so going from rendered view to code view freezes the browser for a second or so.

Maybe we could get the code through a AJAX request? So we still handle it on the frontend, but don't render it initially.

Yeah I really like that benefit as well. But on a IPython notebook file I have locally, it has ~5000 lines, so going from rendered view to code view freezes the browser for a second or so.

Maybe we could get the code through a AJAX request? So we still handle it on the frontend, but don't render it initially.

@iamphill Hmm, yeah, we could do that. If I add a json format to the blob endpoint, could you have a look at that? It will return whatever should go inside .blob-viewer[data-type=simple]. I can of course add that URL in a data-attribute on that .blob-viewer.

@DouweM Yeah I can look at that

added 1 commit

• c0b070db - Added startsWith polyfill

Compare with previous version

@iamphill Can you update the description with a new screenshot now that you've changed the switcher?

added 190 commits

• c0b070db...3082a119 - 189 commits from branch master
• 0f057c2b - Merge branch 'master' into dm-blob-viewers

Compare with previous version

added 1 commit

• 4767c509 - Consistent naming for source/rendered/raw/download

Compare with previous version

added 2 commits

• 414af4e7 - Add JSON endpoint to get simple blob viewer
• 29e847d6 - Allow viewers to override switcher icon and title

Compare with previous version

added 2 commits

• 414af4e7 - Add JSON endpoint to get simple blob viewer
• 29e847d6 - Allow viewers to override switcher icon and title

Compare with previous version

added 1 commit

• 3e4bfeba - Allow max size to be overridden

Compare with previous version

@iamphill if we don't want the freeze the view on bigger files, then we could use a worker. Other than that I don't think there is much else you can do to make it "fast enough" for no jank. Because there will always be a file too big to make it jank.

@jschatz1 Pulling the content down by AJAX reduces a lot of jank, yeah it'll still be there on really large files, but its a massive improvement on what it was for me.

@iamphill yes we could do that and it would reduce jank. I was thinking of the rendering itself. Like the js functions that have to get run to parse the file. For a large file that itself could create jankage.

@jschatz1 This MR doesn't concern itself with the implementation of the individual JS viewers, but using workers sounds like a good idea :)

added 1 commit

• 1f52bcb6 - Use blob viewers on snippets

Compare with previous version

added 5 commits

• b4e422c9 - Fix issue with markdown helper
• 0cbde70e - Use class attributes instead of method overrides for immutable blob viewer properties
• 9ee66c2b - Use BlobViewer instance instead of passing blob as an argument constantly
• 50fcdf77 - Use correct raw URL for snippets
• e9da0715 - Add JSON format to snippet show endpoints

Compare with previous version

Just made it pull the source file async, however i've found a bug with the copy contents button. It doesn't actually work when you click it, it will change to view the source file, but it won't actually copy anything until the element is visible & you click it again. Going to look for a solution to this.

added 1 commit

• e4158aab - Load source file async

Compare with previous version

@iamphill Ah right, it switches and then immediately triggers the copy. We should somehow have that wait until the text is actually loaded. That may be tricky, and we'd need to give clear feedback to te user to that affect so they know it's not copied until the page is loaded.

Ah right, it switches and then immediately triggers the copy. We should somehow have that wait until the text is actually loaded. That may be tricky, and we'd need to give clear feedback to te user to that affect so they know it's not copied until the page is loaded.

It doesn't look like the clipboard library allows for waiting which sucks & it also looks like the user needs to explicitly click the button. I tried triggering it with JS & didn't seem to do it. Will try a few different ways & see if it works.