Refined-github: Restore `faster-pr-diff-options` feature on PRs

Created on 6 Dec 2019  Â·  13Comments  Â·  Source: sindresorhus/refined-github

Originally disabled on PRs because of https://github.com/sindresorhus/refined-github/issues/2291

This feature was applied to two places:

  • PR file header: now disabled because of #2291
  • commit header: it still works

I had suggested some alternatives to replacing the original "PR diff options" dropdown:

  • dropping its "apply and reload" button (saves one click)
  • perhaps opening the dropdown on hover (saves one more click)

There's already a keyboard shortcut to toggle the whitespace.
_Originally posted by @fregante in https://github.com/sindresorhus/refined-github/issues/2291#issuecomment-517539452_

Please! ♥︎ enhancement help wanted under discussion

Most helpful comment

I use the "No Whitespace" button all the time (wish it was just ALWAYS enabled by default) and really miss it now.

All 13 comments

I use the "No Whitespace" button all the time (wish it was just ALWAYS enabled by default) and really miss it now.

Sadly there's not really space for it now. GitHub has too many widgets.

And when you scroll, there's even less space since the PR title is added:

We could change the label on the button from "No whitespace" to something
like "-w" or an icon

On Tue, Dec 17, 2019, 15:57 Federico Brigante notifications@github.com
wrote:

Sadly there's not really space for it:

https://user-images.githubusercontent.com/1402241/62343178-ec00eb00-b513-11e9-9f55-30cb56fc2712.png

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/sindresorhus/refined-github/issues/2597?email_source=notifications&email_token=AAI2U3OQNZCF6KA2B7DB4M3QZE4MPA5CNFSM4JW4L46KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHD5V7I#issuecomment-566745853,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAI2U3PR453AIXLG6KN6UILQZE4MPANCNFSM4JW4L46A
.

Still not enough space if all of GitHub’s widgets are visible and the title is there too

At what screen width does it break?

All of them. Look at the last screenshot I posted. Add the “Commit suggestions” widget and voila: no space left.

As a compromise, it could hide the 'files viewed' indicator maybe? Or we could reduce the amount of space the title has. Or we would increase the width the content has on the header.
I'm happy to mock up visualisations of these if you're interested.

it could hide the 'files viewed' indicator maybe?

We shouldn't hide a useful feature (the feature's utility is debatable, but...)

Or we could reduce the amount of space the title has.

I don't wanna play that game. We tried it for the past 2 years and we've been just moving the problem and never resolving it.

Or we would increase the width the content has on the header.

This could work, but the compromises are:

  • only works when the window fits it
  • it might not look great since the header would be _slightly longer_ than the content width

In the end, I think these are the safest to implement and they still bring down the total number of clicks to one (+ hover)

  • dropping its "apply and reload" button (saves one click)
  • opening the dropdown on hover (saves one more click)

+1 for dropping apply and reload, especially as only two settings live on that menu. Not sure about the latter, is the hover behaviour implemented anywhere else?

is the hover behaviour implemented anywhere else?

It used to be implemented on the reactions popup, but it stopped working and we never readded it.

Can you at least re-add the shortcut? :slightly_smiling_face:

PR welcome for that. Most of the feature code can be dropped: https://github.com/sindresorhus/refined-github/blob/master/source/features/faster-pr-diff-options.tsx

Something as short as:

function init(): void {
    const searchParameters = new URLSearchParams(location.search);
    const isHidingWhitespace = searchParameters.get('w') === '1';

    if (isHidingWhitespace) {
        searchParameters.delete('w');
    } else {
        searchParameters.set('w', '1');
    }

    document.body.append(
        <a
            classNames="sr-only"
            aria-label={`Show ${type} diffs`}
            href={`?${String(parameters)}`}
        />
    );
}
Was this page helpful?
0 / 5 - 0 ratings