Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reaction tooltips missing in GitHub #22

Closed
Vangelis66 opened this issue Jun 7, 2022 · 20 comments
Closed

Reaction tooltips missing in GitHub #22

Vangelis66 opened this issue Jun 7, 2022 · 20 comments
Labels
std-tech Problem caused by missing technology, interface etc.

Comments

@Vangelis66
Copy link

Many thanks for this most valuable extension 👍 .
I'm currently in the process of migrating from gh-wc-polyfill to palefill, but stumbled across a bug/irritation:

STR
Version of extension: 1.11

  1. Load, e.g.,
    Update to PM 31.0 #8 (comment)
  2. Hover over the reaction emoji, to see the identity of the GH-member that reacted
  3. Expected behaviour: A tooltip pops-up with the GH-username (among other things):

rt1

This is currently working as expected with gh-wc-polyfill-v1.2.19 😉

  1. Actual behaviour: No tooltip pops up when the reaction emoji is hovered over... 😞

I can also reproduce with latest palefill ALL the problems reported in upstream issue #54: JustOff/github-wc-polyfill#54

problems fixed by
JustOff/github-wc-polyfill@c0f5ff3
and
JustOff/github-wc-polyfill@74179c2

Thanks in advance for any eventual resolution... 😄

@martok martok added the std-tech Problem caused by missing technology, interface etc. label Jun 8, 2022
@martok martok closed this as completed in 58a1a29 Jun 8, 2022
@Vangelis66
Copy link
Author

"Vielen Dank" for your prompt action! 👍
I compiled from the master branch, source snapshot martok-palefill-v1.11-3-g58a1a29, and it's working splendidly:

pf2

@martok
Copy link
Owner

martok commented Jul 31, 2022

That's actually Github screwing it up. The elements get the dropdown-menu-w class (which usually means "west"), and that is excactly where they show up. They should set dropdown-menu-e (for "east").

@Vangelis66
Copy link
Author

Vangelis66 commented Aug 1, 2022

That's actually Github screwing it up.

Yes 😠 ... I can easily reproduce with a recent-ish Chromium-based browser, too 😞 ...
In the meantime, could Microsoft's blunder be mitigated somehow via a userstyle or userscript?
Currently, the only way to react (but NOT on Releases) is via the "emoji" button in the comment's header (which also spawns a pop-up to its (downwards) left) ...

GH

@Vangelis66
Copy link
Author

Vangelis66 commented Aug 6, 2022

How typical of Microsoft... 😠 😡

The misplaced "reaction emoji selection panel" reported by @NEO-SPECTRUMAN appears to have been now fixed:

  1. reacting on a comment:
    REACTION1
  2. reacting on a release:
    REACTION

... however, this original issue has now come back with a vengeance 😡,
i.e.: hovering over already cast reaction emojis (under comments/releases) doesn't spawn a pop-up anymore with the usernames of the GH members that reacted, much like in my OP here...
This time GH did something else, because the old upstream extension, gh-wc-polyfill-v1.2.19, no longer works in that regard...

This issue should be re-opened and revisited...
FWIW, Serpent v52.9.0 (2022-07-30) (32-bit) and latest stable palefill (v1.18);
@martok, please, kindly to the rescue! ❤️

Later EDIT: I also forgot to note that ALL GH tooltips are now seemingly affected (i.e. have vanished), including GH Editor's toolbar buttons (like when typing/editing this very comment):

missingtooltips

@martok martok reopened this Aug 6, 2022
@SeaHOH
Copy link

SeaHOH commented Aug 7, 2022

Patches below fixed the new issue.

palefill/lib/polyfills.js

Lines 333 to 350 in 2d141c1

if (context === undefined && selectors === undefined)
return tagName;
if (context)
return \`:-moz-any(\${selectors}) \${tagName}\`;
let res = [];
for (let selector of selectors.split(","))
res.push(tagName + selector);
return \`:-moz-any(\${res.join(", ")})\`;
}) + style;
}
}
}
};
const asNames = new Set(["article", "aside", "blockquote", "body", "div",
"footer", "h1", "h2", "h3", "h4", "h5", "h6",
"header", "main", "nav", "p", "section", "span"]);
Element.prototype.attachShadow = function attachShadow(init) {
if (this.shadowRoot !== undefined)

                  return !context ? !selectors ? tagName :
                      \`\${tagName}:-moz-any(\${selectors})\` :
                      \`:-moz-any(\${selectors}) \${tagName}\`;
                }) + style;
          }
        }
      }
    };
    const observer = new MutationObserver(function(mutations) {
            for (const {target, attributeName, oldValue} of mutations) {
              target.attributeChangedCallback(attributeName, oldValue, target.getAttribute(attributeName));
            }
          }),
          asNames = new Set(["article", "aside", "blockquote", "body", "div",
                             "footer", "h1", "h2", "h3", "h4", "h5", "h6",
                             "header", "main", "nav", "p", "section", "span"]);
    Element.prototype.attachShadow = function attachShadow(init) {
      // customElements observe Shadow failed, so re-start observe here
      const {observedAttributes} = this.__CE_definition || {};
      if (observedAttributes) {
        observer.observe(this, {
          attributes: true,
          attributeOldValue: true,
          attributeFilter: observedAttributes
        });
      }
      if (this.shadowRoot !== undefined)

@martok
Copy link
Owner

martok commented Aug 8, 2022

Makes sense, that fits with how far I managed to narrow the problem down.

Thank you, will apply later!

@Vangelis66
Copy link
Author

Vangelis66 commented Aug 9, 2022

@SeaHOH

Between 202208072241Z and 202208090651Z, you have published, in total, nine (9) different iterations of #22 patches, so many thanks, indeed 👍 ...

The date/time while writing this comment is ca. 202208091700Z; during the last hour or so, I have been applying each and every one of these 9 different "patches" of yours, and then compiling them locally into XPI files, then testing each resultant XPI extension file...

Do you know what the current outcome is?
Each and every one of these 9 different patches, if applied on top of source snapshot martok-palefill-v1.18-2-git-20220803-g2d141c1, produces, in the end, an .XPI file which, when installed in the browser [St52 v52.9.0 (2022-08-05) (32-bit)], BREAKS GitHub in my UXP-based browser 😞 ...

I have been very careful at properly applying the patches onto file polyfills.js of snapshot 2d141c1, so, hopefully, it isn't something caused by me messing up things (but I can never rule it out 😜 ...).

Latest palefill release (v1.18) as well as latest master HEAD (palefill-v1.18-2-git-20220803-g2d141c1) when compiled to an XPI DO NOT break GitHub, but, alas, don't have a fix for this issue...

TL;DR: As of this time, proposed "fixes" do not remedy this issue... 😭

@SeaHOH
Copy link

SeaHOH commented Aug 9, 2022

One line has been missed, it's OK now.

@Vangelis66
Copy link
Author

... Thanks for your prompt reply 😄 !
I have applied the most up-to-date version of your patch,

--- a.polyfills.js      2022-08-04 00:32:21.000000000 +0300
+++ b.polyfills.js      2022-08-09 21:14:20.477000000 +0300
@@ -330,23 +330,32 @@
                 // flag "s" is broken in SeaMonkey
                 /:host(-context)?(?:\\(([\\s\\S]+?)\\))?/g,
                 function ($, context, selectors) {
-                  if (context === undefined && selectors === undefined)
-                    return tagName;
-                  if (context)
-                    return \`:-moz-any(\${selectors}) \${tagName}\`;
-                  let res = [];
-                  for (let selector of selectors.split(","))
-                    res.push(tagName + selector);
-                  return \`:-moz-any(\${res.join(", ")})\`;
+                  return !context ? !selectors ? tagName :
+                      \`\${tagName}:-moz-any(\${selectors})\` :
+                      \`:-moz-any(\${selectors}) \${tagName}\`;
                 }) + style;
           }
         }
       }
     };
-    const asNames = new Set(["article", "aside", "blockquote", "body", "div",
+    const observer = new MutationObserver(function(mutations) {
+            for (const {target, attributeName, oldValue} of mutations) {
+              target.attributeChangedCallback(attributeName, oldValue, target.getAttribute(attributeName));
+            }
+          }),
+          asNames = new Set(["article", "aside", "blockquote", "body", "div",
                              "footer", "h1", "h2", "h3", "h4", "h5", "h6",
                              "header", "main", "nav", "p", "section", "span"]);
     Element.prototype.attachShadow = function attachShadow(init) {
+      // customElements observe Shadow failed, so re-start observe here
+      const {observedAttributes} = this.__CE_definition || {};
+      if (observedAttributes) {
+        observer.observe(this, {
+          attributes: true,
+          attributeOldValue: true,
+          attributeFilter: observedAttributes
+        });
+      }
       if (this.shadowRoot !== undefined)
         throw new DOMException(
             \`The <\${this.tagName}> element has be tried to attach to is already a shadow host.\`,

compiled locally source to .XPI, installed and can verify now things are OK:

proof

@martok
Copy link
Owner

martok commented Aug 10, 2022

Thank you two!
I've moved the observer attachment down below the input validation, otherwise applied unchanged.

@martok martok closed this as completed Aug 10, 2022
@Vangelis66
Copy link
Author

Vangelis66 commented Aug 15, 2022

reaction popup panel appears on left corner of button and goes out of screen

That's actually Github screwing it up.

The misplaced "reaction emoji selection panel" reported by @NEO-SPECTRUMAN
appears to have been now fixed:

😡 ; just a short while ago, I bumped into one more such instance which hasn't been fixed; it's "Conversations" as part of PRs...

STR

  1. Navigate to
    [jsinterp] Overhaul JSInterp to handle new YT players 4c3f79c5, 324f67b9 ytdl-org/youtube-dl#31170
    (the "Conversations" tab should have been selected by default)
  2. Try to react to the opening post by clicking on the "emoji" button at the bottom of that post/comment; result:

GH1

Really? How incompetent are they at Microsoft currently? 👎 😞
Anyhow, only way to react is by using the "emoji" button in the post's header...

@dirkf
Copy link

dirkf commented Aug 15, 2022

UserCSS:

.dropdown-menu-reactions.dropdown-menu-w {
	float: left;
	left: 0px;
        top: -180%;
	width: -moz-fit-content; /* or 1000% */
        /* uncomment for specific bg colour
        background-color: black;
        */
}

For browsers that we don't care about: https://stackoverflow.com/questions/7839164/is-there-a-css-cross-browser-value-for-width-moz-fit-content.

@Vangelis66
Copy link
Author

Vangelis66 commented Aug 15, 2022

Thanks for that userstyle 😄 ; I've tried it, and while it does now allow for (un-)reacting, it appears as an "ugly" kludge (which is still better than nothing 😉 ) in my case:

ghreact1

(among other things, the panel's background is transparent, etc.).

Additionally, I'd need someone to kindly offer the regex needed for the userstyle to be applied selectively on
https://github.com/*/*/pull/* URIs, as other "emoji" buttons (under issue comments and releases) are not affected by MS's blunder 😠 ... As you might've guessed by now, neither CSS nor RegExp is my forte... 😞

@dirkf
Copy link

dirkf commented Aug 15, 2022

I suspect this depends on your theme in GH. Mine is "Light high contrast". Insert a line with background-color: black; in the {...} to get one.

See the updated CSS above for a more specific fix that doesn't affect the other reaction menus. When applied with Stylem the CSS properties need the !important attribute: eg float: left !important;.

@Vangelis66
Copy link
Author

See the updated CSS above for a more specific fix
that doesn't affect the other reaction menus.

Your altruistic help is being highly valued here! ❤️
Your edited/updated code works brilliantly for me now:

dirkf-fix1

I just tweaked somewhat the top: parameter to a -225% value, so it more closely resembles the behaviour of the other "emoji-buttons" (makes the panel display slightly further above).

When applied with Stylem

In my case, my current browser (latest Serpent 52.9.0) has kept Fx-52esr-level support for Web Extensions, so I'm using Stylus-v1.4.23 as a userstyle manager - contrary to Stylem, Stylus has additional support for *.user.css userstyles (the type that can be further configured by the user post-install); I've been waiting/hoping for that support to arrive in Stylem, too, but I fear that ship has sailed (and sunk?).

I suspect this depends on your theme in GH. Mine is "Light high contrast"

I, OTOH, use Dark default, together with the excellent Dark GitHub userstyle (and yes, it's of the *.user.css format 😉 ).

Kindest regards 😄

@Vangelis66
Copy link
Author

Vangelis66 commented Aug 15, 2022

... And... UPDATE (this is a bottomless pit 😡 ) !

The praise-worthy solution by dirkf apparently breaks another "emoji-button":
the one appearing in comments inside actual commit code (there's a proper term for these comments, probably, but I hope you get my drift...). Visit:

ytdl-org/youtube-dl@d231b56#r81169570

; if you click on the "emoji-button" to react to pukkandan's comment, the emoji selection popup is displayed to the right, but truncated 😞 :

dirkf-fix2

Fortunately, temporarily disabling dirkf's userstyle in Stylus will allow for that (marginal) reaction case to function; I have currently the userstyle enabled on domain github.com, but, as I said previously, some fine-tuning via RegExp to restrict it to PR Conversations is, apparently, still needed...

Later addition:
It would appear the below RegExp filter for CSS does it for me:

https://github\.com/.*/.*/pull/.*

Got help from this and that 👍 ...

@SeaHOH
Copy link

SeaHOH commented Aug 16, 2022

just adjust the selector as .comment-reactions .dropdown-menu-reactions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
std-tech Problem caused by missing technology, interface etc.
Projects
None yet
Development

No branches or pull requests

5 participants
@martok @dirkf @SeaHOH @Vangelis66 and others