Skip to content

default-branch-button - Avoid duplicate buttons#9148

Closed
SunsetTechuila wants to merge 2 commits into
mainfrom
default-branch-button
Closed

default-branch-button - Avoid duplicate buttons#9148
SunsetTechuila wants to merge 2 commits into
mainfrom
default-branch-button

Conversation

@SunsetTechuila
Copy link
Copy Markdown
Member

@SunsetTechuila SunsetTechuila commented Apr 3, 2026 •
edited
Loading

@SunsetTechuila SunsetTechuila force-pushed the default-branch-button branch from 9040e88 to 468ca33 Compare April 3, 2026 13:36
SunsetTechuila
SunsetTechuila commented Apr 3, 2026
View reviewed changes
Comment thread source/features/default-branch-button.tsx Outdated
Comment on lines +45 to +46
const buttonGroup = branchSelector.closest('[class*="ButtonGroup"]') ?? undefined;
const nativeButton = $optional('[type="button"]:has(> .octicon-chevron-left)', buttonGroup);
Copy link
Copy Markdown
Member Author

@SunsetTechuila SunsetTechuila Apr 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Or

const nativeButtonLabel = 'View file on default branch';

const nativeButton = getElementByAriaLabelledBy('a[data-component="IconButton"]', nativeButtonLabel)
    ?? $optional('button[aria-label="${nativeButtonLabel}"]');
  if (nativeButton?.ariaDisabled === 'false') {
    return;
  }

Copy link
Copy Markdown
Member Author

@SunsetTechuila SunsetTechuila Apr 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Disabled button:

HTML 
<div>
  <button
    aria-disabled="true"
    data-component="IconButton"
    type="button"
    aria-label="View file on default branch"
    class="prc-Button-ButtonBase-9n-Xk prc-Button-IconButton-fyge7"
    data-inactive="true"
    data-loading="false"
    data-no-visuals="true"
    data-size="medium"
    data-variant="default"
    aria-describedby="_R_5hlmjal1d_"
  >
    <svg
      aria-hidden="true"
      focusable="false"
      class="octicon octicon-chevron-left"
      viewBox="0 0 16 16"
      width="16"
      height="16"
      fill="currentColor"
      display="inline-block"
      overflow="visible"
      style="vertical-align:text-bottom"
    >
      <path d="M9.78 12.78a.75.75 0 0 1-1.06 0L4.47 8.53a.75.75 0 0 1 0-1.06l4.25-4.25a.751.751 0 0 1 1.042.018.751.751 0 0 1 .018 1.042L6.06 8l3.72 3.72a.75.75 0 0 1 0 1.06Z"></path>
    </svg>
  </button>
  <span
    class="prc-TooltipV2-Tooltip-tLeuB"
    data-direction="se"
    role="tooltip"
    aria-hidden="true"
    id="_R_5hlmjal1d_"
    popover="auto"
    style="top: 200px; left: 16px;"
  >
    File does not exist on default branch
  </span>
</div>;

Enabled:

HTML 
<div>
  <a
    data-component="IconButton"
    type="button"
    class="prc-Button-ButtonBase-9n-Xk prc-Button-IconButton-fyge7"
    data-loading="false"
    data-no-visuals="true"
    data-size="medium"
    data-variant="default"
    aria-labelledby="_R_5hlmjal1d_"
    href="/refined-github/refined-github/blob/main/.ncurc.json"
    data-discover="true"
  >
    <svg
      aria-hidden="true"
      focusable="false"
      class="octicon octicon-chevron-left"
      viewBox="0 0 16 16"
      width="16"
      height="16"
      fill="currentColor"
      display="inline-block"
      overflow="visible"
      style="vertical-align: text-bottom;"
    >
      <path d="M9.78 12.78a.75.75 0 0 1-1.06 0L4.47 8.53a.75.75 0 0 1 0-1.06l4.25-4.25a.751.751 0 0 1 1.042.018.751.751 0 0 1 .018 1.042L6.06 8l3.72 3.72a.75.75 0 0 1 0 1.06Z"></path>
    </svg>
  </a>
  <span
    class="prc-TooltipV2-Tooltip-tLeuB"
    data-direction="se"
    aria-hidden="true"
    id="_R_5hlmjal1d_"
    popover="auto"
    style="top: 200px; left: 16px;"
  >
    View file on default branch
  </span>
</div>;

@SunsetTechuila SunsetTechuila marked this pull request as ready for review April 3, 2026 13:44
SunsetTechuila
SunsetTechuila commented Apr 3, 2026
View reviewed changes
Comment thread source/features/default-branch-button.tsx Outdated
async function add(branchSelector: HTMLElement): Promise<void> {
const buttonGroup = branchSelector.closest('[class*="ButtonGroup"]') ?? undefined;
const nativeButton = $optional('[type="button"]:has(> .octicon-chevron-left)', buttonGroup);
if (nativeButton?.ariaDisabled !== 'true') {
Copy link
Copy Markdown
Member Author

@SunsetTechuila SunsetTechuila Apr 3, 2026

Choose a reason for hiding this comment

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

Enabled button doesn't have the "aria-disabled" attribute

@SunsetTechuila SunsetTechuila force-pushed the default-branch-button branch from 72023f3 to 57f3246 Compare April 3, 2026 13:58
fregante
fregante approved these changes Apr 3, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@fregante fregante left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Let's also change the tooltip to "View on default branch" in this PR. It matches the native wording except we don't have to alternate between files and folders.

@SunsetTechuila SunsetTechuila marked this pull request as draft April 3, 2026 14:47
@SunsetTechuila
Copy link
Copy Markdown
Member Author

SunsetTechuila commented Apr 3, 2026

No, this should be done with CSS

@SunsetTechuila
Copy link
Copy Markdown
Member Author

SunsetTechuila commented Apr 3, 2026

msedge_lAEkW7R09X

@fregante
Copy link
Copy Markdown
Member

fregante commented Apr 16, 2026

I'd love to see this before the next version

@SunsetTechuila
Copy link
Copy Markdown
Member Author

SunsetTechuila commented Apr 17, 2026

I'd love to see this before the next version

When do you plan to release it?

@fregante
Copy link
Copy Markdown
Member

fregante commented Apr 17, 2026

I guess we gotta do 4.20

@the-corp-mark the-corp-mark mentioned this pull request Apr 17, 2026
Closed
@SunsetTechuila
Copy link
Copy Markdown
Member Author

SunsetTechuila commented Apr 20, 2026 •
edited
Loading

The feature actually stopped working on most of the pages because branchSelector needs to be updated

@SunsetTechuila
Copy link
Copy Markdown
Member Author

SunsetTechuila commented Apr 20, 2026

[id^="ref-picker-"] likely will do

@fregante
Copy link
Copy Markdown
Member

fregante commented Apr 30, 2026 •
edited
Loading

The feature seems to be broken altogether in all RGH versions :(

@fregante
Copy link
Copy Markdown
Member

fregante commented May 7, 2026

Closing until the feature is restored

@fregante fregante closed this May 7, 2026
@SunsetTechuila SunsetTechuila deleted the default-branch-button branch May 7, 2026 15:17
@SunsetTechuila
Copy link
Copy Markdown
Member Author

SunsetTechuila commented May 7, 2026

No, this should be done with CSS

All changes had to be reverted anyway

@SunsetTechuila SunsetTechuila mentioned this pull request May 10, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fregante fregante fregante approved these changes

Assignees

No one assigned

Labels

bug

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

default-branch-button implemented by GitHub

2 participants

@SunsetTechuila @fregante