Refined-github: Restore `default-to-rich-diff` feature

Created on 14 May 2019  路  7Comments  路  Source: sindresorhus/refined-github

Hi!

The problem:
url
The URL to reproduce:
https://github.com/xcp-ng/xcp-ng-release/pull/7/files

Thanks :)

bug help wanted

Most helpful comment

It can be there, disabled and with a tooltip, like other similar non-working buttons

All 7 comments

That's probably because of default-to-rich-diff feature.

Just by looking at the code, my assumption is that for deleted svg files GitHub doesn't render a .render-wrapper, causing both the conditions on https://github.com/sindresorhus/refined-github/blob/09d1ab67b8eae588f33d4f8aa31cfeec1f9dac93/source/features/default-to-rich-diff.tsx#L19 to always fail, which in turn causes the interval to keep on clicking the button for every 300 milliseconds.

Indeed this needs to skip deleted files. Thanks for the report!

It's kinda weird that GitHub displays the Rich Diff button even if it won't work

It's kinda weird that GitHub displays the Rich Diff button even if it won't work

Not having it would me more confusing IMO, having it provides a reason on why the rich diff cannot be rendered.

It can be there, disabled and with a tooltip, like other similar non-working buttons

A bigger bug here is that the page scrolls down to the file. If that happens all the time we probably have to drop the feature

A bigger bug here is that the page scrolls down to the file. If that happens all the time we probably have to drop the feature

Not necessarily, what if we make the request to and update the DOM instead of clicking the button. GitHub currently does innerHTML for this I'm assuming we can use <include-fragment> too.

If this is a big issue, then we can drop it (Also saving serve costs for GitHub 馃構).

Yeah this feature needs to be rewritten or dropped because the scroll jump is seen everywhere. I looked into their code, this is what I found:

Starting from here:
https://github.com/elementary/icons/pull/792/files

The first file loads this:
https://github.com/elementary/icons/diffs/0?base_sha=8333483bdfaf300e175c285bed70203a283c2230&commentable=true&head_user=lainsce&pull_number=792&sha1=8333483bdfaf300e175c285bed70203a283c2230&sha2=50415810f666efb4e03f8710999c08e09d441d46&short_path=0321141

That URL is found in the button's form. We can load that with fetchDom and inject it. That pages includes the whole loading gif interface that loads this in an iframe:
https://render.githubusercontent.com/diff/svg?commit=8333483bdfaf300e175c285bed70203a283c2230&enc_url1=68747470733a2f2f7261772e67697468756275736572636f6e74656e742e636f6d2f656c656d656e746172792f69636f6e732f383333333438336264666166333030653137356332383562656437303230336132383363323233302f616374696f6e732f31362f77696e646f772d636c6f73652e737667&enc_url2=68747470733a2f2f7261772e67697468756275736572636f6e74656e742e636f6d2f656c656d656e746172792f69636f6e732f353034313538313066363636656662346530336638373130393939633038653039643434316434362f616374696f6e732f31362f77696e646f772d636c6f73652e737667&path=actions%2F16%2Fwindow-close.svg&repository_id=47588124#69c770a3-32de-4dc2-9497-586d7628242b

So for every SVG it's two HTTP requests. I don't think we can skip this intermediary page, but if we could it'd be great.

At the very least, for every SVG we should load the intermediary page and change the button classes.

cc @idasbiste

Was this page helpful?
0 / 5 - 0 ratings

Related issues

fregante picture fregante  路  3Comments

supremebeing7 picture supremebeing7  路  3Comments

sompylasar picture sompylasar  路  3Comments

juliocanares picture juliocanares  路  3Comments

hkdobrev picture hkdobrev  路  3Comments