`reactions-avatars` - Fix feature in locked conversation by kovsu · Pull Request #8083 · refined-github/refined-github

kovsu · 2024-11-19T01:30:07Z

Close: #8081

Test URLs

PR: Help users find a feature with an interactive binary search #4119
Locked PR: Add "Wait for successful checks" checkbox #975
Discussion: Build error when using JS imports: Browser scripts cannot have imports or exports parcel-bundler/parcel#6490
Locked discussion: https://github.com/orgs/community/discussions/28776

Screenshot

Google.Chrome.mp4

fregante · 2024-11-19T04:02:21Z

This might break discussions: #7930 (comment)

Does ee7ce12 (#7930) need to be reverted?

What happens with this PR in discussions?

kovsu · 2024-11-19T05:59:03Z

Maybe we should apply this #7930 (comment)

fregante · 2024-11-19T06:04:49Z

Does ee7ce12 (#7930) need to be reverted?

Yes this was it, unless this PR works on discussions as is (without errors).

For the record, it seems that locked discussions can still be reacted to. We should add this to the test URLs: https://github.com/orgs/community/discussions/28776

fregante · 2024-11-19T06:08:39Z

this PR works on discussions as is (without errors).

Did you see any errors on the previous version of this PR? The selector seems to work ok without the last commit.

kovsu · 2024-11-19T06:11:37Z

Did you see any errors on the previous version of this PR? The selector seems to work ok without the last commit.

It has error if i remove the enabled. So I revert it and it doesn't.

kovsu · 2024-11-19T06:13:12Z

I update the screenshot with the last code.

fregante · 2024-11-19T06:15:24Z

Did you see any errors on the previous version of this PR? The selector seems to work ok without the last commit.

It has error if i remove the enabled. So I revert it and it doesn't.

What error and on what page? I'm running the previous commit and I see no errors on https://github.com/orgs/community/discussions/28776

We should add this to the test URLs:

By this I generally mean in the file itself, for future reference.

fregante · 2024-11-19T06:16:08Z

What error and on what page? I'm running the previous commit and I see no errors on github.com/orgs/community/discussions/28776

Oh I'm seeing it on https://github.com/orgs/community/discussions/30093, we should add this to the test URLs in the file.

kovsu · 2024-11-19T06:20:43Z

Done

fregante · 2024-11-19T06:23:38Z

I think I know what's happening. GitHub loads a disabled/static version of the reactions and then it replaces it with an enabled/active version that has the tool-tip element.

<div
  class="js-comment-reactions-options d-flex flex-items-center flex-row flex-wrap rgh-seen--8573360097"
>
  <button
    name="input[content]"
    id="reactions--reaction_button_component-68cd63"
    value="THUMBS_DOWN react"
    data-button-index-position="1"
    data-reaction-label="-1"
    data-reaction-content="-1"
    aria-pressed="false"
    type="submit"
    disabled="disabled"
    data-view-component="true"
    class="social-reaction-summary-item js-reaction-group-button btn-link d-flex no-underline color-fg-muted flex-items-baseline"
  >
    <g-emoji
      alias="-1"
      fallback-src="https://github.githubassets.com/assets/1f44e-ce91733aae25.png"
      class="social-button-emoji"
      >👎</g-emoji
    >
    <span>3</span>
  </button>
  <button
    name="input[content]"
    id="reactions--reaction_button_component-e6663f"
    value="CONFUSED react"
    data-button-index-position="4"
    data-reaction-label="Confused"
    data-reaction-content="thinking_face"
    aria-pressed="false"
    type="submit"
    disabled="disabled"
    data-view-component="true"
    class="social-reaction-summary-item js-reaction-group-button btn-link d-flex no-underline color-fg-muted flex-items-baseline"
  >
    <g-emoji
      alias="thinking_face"
      fallback-src="https://github.githubassets.com/assets/1f615-4bb1369c4251.png"
      class="social-button-emoji"
      >😕</g-emoji
    >
    <span>1</span>
  </button>
</div>

<div
  class="js-comment-reactions-options d-flex flex-items-center flex-row flex-wrap rgh-seen--8573360097"
>
  <button
    name="input[content]"
    id="reactions--reaction_button_component-efbc2d"
    value="THUMBS_DOWN react"
    data-button-index-position="1"
    data-reaction-label="-1"
    data-reaction-content="-1"
    aria-pressed="false"
    aria-label="react with thumbs down"
    type="submit"
    data-view-component="true"
    class="social-reaction-summary-item js-reaction-group-button btn-link d-flex no-underline color-fg-muted flex-items-baseline mr-2"
    aria-describedby="tooltip-b5f4c3ef-506b-4a65-bf00-3f46881ea683"
  >
    <g-emoji
      alias="-1"
      fallback-src="https://github.githubassets.com/assets/1f44e-ce91733aae25.png"
      class="social-button-emoji"
      >👎</g-emoji
    >
    <span class="js-discussion-reaction-group-count">3</span>
    <span class="avatar-user avatar rgh-reactions-avatar p-0 flex-self-center"
      ><img
        src="https://avatars.githubusercontent.com/Googologyer?size=32"
        class="d-block"
        width="16"
        height="16"
        alt="@Googologyer"
        loading="lazy" /></span
    ><span class="avatar-user avatar rgh-reactions-avatar p-0 flex-self-center"
      ><img
        src="https://avatars.githubusercontent.com/VasanthGN?size=32"
        class="d-block"
        width="16"
        height="16"
        alt="@VasanthGN"
        loading="lazy" /></span
    ><span class="avatar-user avatar rgh-reactions-avatar p-0 flex-self-center"
      ><img
        src="https://avatars.githubusercontent.com/u/33871030?s=64&amp;v=4"
        class="d-block"
        width="16"
        height="16"
        alt="@Generalsimus"
        loading="lazy"
    /></span>
  </button>
  <tool-tip
    id="tooltip-b5f4c3ef-506b-4a65-bf00-3f46881ea683"
    for="reactions--reaction_button_component-efbc2d"
    popover="manual"
    data-direction="n"
    data-type="description"
    data-view-component="true"
    class="position-absolute sr-only"
    role="tooltip"
    style="--tool-tip-position-top: 657.515625px; --tool-tip-position-left: 150.57421875px;"
    >Googologyer, VasanthGN, and Generalsimus reacted with thumbs down
    emoji</tool-tip
  >
  <button
    name="input[content]"
    id="reactions--reaction_button_component-338e4a"
    value="CONFUSED react"
    data-button-index-position="4"
    data-reaction-label="Confused"
    data-reaction-content="thinking_face"
    aria-pressed="false"
    aria-label="react with confused"
    type="submit"
    data-view-component="true"
    class="social-reaction-summary-item js-reaction-group-button btn-link d-flex no-underline color-fg-muted flex-items-baseline mr-2"
    aria-describedby="tooltip-87cd2e93-78f1-481a-b2e3-b2432ea6925d"
  >
    <g-emoji
      alias="thinking_face"
      fallback-src="https://github.githubassets.com/assets/1f615-4bb1369c4251.png"
      class="social-button-emoji"
      >😕</g-emoji
    >
    <span class="js-discussion-reaction-group-count">1</span>
    <span class="avatar-user avatar rgh-reactions-avatar p-0 flex-self-center"
      ><img
        src="https://avatars.githubusercontent.com/wrightdoesphotography?size=32"
        class="d-block"
        width="16"
        height="16"
        alt="@wrightdoesphotography"
        loading="lazy"
    /></span>
  </button>
  <tool-tip
    id="tooltip-87cd2e93-78f1-481a-b2e3-b2432ea6925d"
    for="reactions--reaction_button_component-338e4a"
    popover="manual"
    data-direction="n"
    data-type="description"
    data-view-component="true"
    class="sr-only position-absolute"
    role="tooltip"
    >wrightdoesphotography reacted with confused emoji</tool-tip
  >
  <div class="js-reactions-container">
    <details
      class="dropdown details-reset details-overlay d-inline-block js-all-reactions-popover"
      hidden=""
    >
      <summary
        aria-haspopup="true"
        data-view-component="true"
        class="Button--link Button--medium Button"
      >
        <span class="Button-content">
          <span class="Button-label">All reactions</span>
        </span>
      </summary>

      <ul class="dropdown-menu dropdown-menu-se">
        <li
          class="dropdown-item"
          aria-label="Googologyer, VasanthGN, and Generalsimus reacted with thumbs down emoji"
        >
          <g-emoji
            alias="-1"
            fallback-src="https://github.githubassets.com/assets/1f44e-ce91733aae25.png"
            class="social-button-emoji mr-2"
            >👎</g-emoji
          >
          <span>3 reactions</span>
        </li>
        <li
          class="dropdown-item"
          aria-label="wrightdoesphotography reacted with confused emoji"
        >
          <g-emoji
            alias="thinking_face"
            fallback-src="https://github.githubassets.com/assets/1f615-4bb1369c4251.png"
            class="social-button-emoji mr-2"
            >😕</g-emoji
          >
          <span>1 reaction</span>
        </li>
      </ul>
    </details>
  </div>
</div>

The way I'd deal with this is by watching specifically the tool-tip element in one case [role=switch] in the other, because currently the feature is processing a bunch of elements that don't need processing.

I'll try something

fregante · 2024-11-19T07:00:18Z

Test URLs updated and described
"temporarily-disabled" buttons excluded from the main observer
Dropped outdated/unnecessary complexity (4-year-old bug fixes that are no longer needed)
Kept the tool-tip check and turned the lack of it into a proper error. We do not want features that fail silently

I checked all the test URLs and it works everywhere without errors

fregante

works for me if it works for you

fregante · 2024-11-19T07:02:06Z

It doesn't work in the new issues 😅

fregante · 2024-11-19T07:17:39Z

Ok we should be good to go.

By the way you can now create branches from this repository. This might help to work on PRs that are not based on old commits

reactions-avatars - Fix feature in locked conversation

be39979

fregante added the bug label Nov 19, 2024

update

7455a3e

comment

6291c95

Drop years old cruft

cefd505

fregante self-assigned this Nov 19, 2024

Properly exclude reactions that don't have participants yet

3b8ace8

fregante approved these changes Nov 19, 2024

View reviewed changes

fregante marked this pull request as draft November 19, 2024 07:01

fregante added 2 commits November 19, 2024 14:16

Restore React view support; add releases test url

9cc949c

Merge remote-tracking branch 'origin/main' into pr/8083

e187b52

fregante marked this pull request as ready for review November 19, 2024 07:16

fregante merged commit f354c9a into refined-github:main Nov 21, 2024

kovsu deleted the fix-#8081 branch November 21, 2024 05:56

Uh oh!

Conversation

kovsu commented Nov 19, 2024 • edited by fregante Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test URLs

Screenshot

Uh oh!

fregante commented Nov 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kovsu commented Nov 19, 2024

Uh oh!

fregante commented Nov 19, 2024

Uh oh!

fregante commented Nov 19, 2024

Uh oh!

kovsu commented Nov 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kovsu commented Nov 19, 2024

Uh oh!

fregante commented Nov 19, 2024

Uh oh!

fregante commented Nov 19, 2024

Uh oh!

kovsu commented Nov 19, 2024

Uh oh!

fregante commented Nov 19, 2024

Uh oh!

fregante commented Nov 19, 2024

Uh oh!

fregante left a comment

Choose a reason for hiding this comment

Uh oh!

fregante commented Nov 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fregante commented Nov 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

kovsu commented Nov 19, 2024 •

edited by fregante

Loading

fregante commented Nov 19, 2024 •

edited

Loading

kovsu commented Nov 19, 2024 •

edited

Loading

fregante commented Nov 19, 2024 •

edited

Loading