Observe subtree of dashboard news to fix hide-own-stars

[notlmn]

Should probably fix #1619.

I could not test this for myself, but it should work. 🤞

There may be an easy of implementing this feature while using CSS using the :has() selector, which (as of now) is not implemented by any of the browsers.

[fregante]

I don’t see how the two are related (although I haven’t tested this) and I’d really prefer not using subtree

[notlmn]

I don’t see how the two are related

These incidents are related, in the sense we are talking about the dashboard DOM that has the load-more button.

I don't actually remember what the structure of dashboard DOM was before they added the recent activity section on the top.

Maybe it was just news items inside the news section.

#dashboard
	.news
		.watch_started
		.fork
		.push
		.public
		...
		... // new items appended here

If this were the case (which is what I remember to be) the mutation observer was listening for only mutations on its immediate child elements, which worked as expected.

But the current DOM is a but different.

#dashboard
	.news
		div[data-repository-hovercards-enabled]
			.watch_started
			.fork
			.push
			.public
			...
			... // new items appended here

And now mutations happen inside the div here (except for the first mutation event, which happens when this div is appended to .news), and from there no more mutation events are generated on the .news node because mutations do not happen on its immediate child nodes, but on its subtree.

So adding subtree is required, or we could move mutation observer from .news to .news > div[data-repository-hovercards-enabled] or .news > div:last-child, which is very prone to break anytime GH decides to changes the DOM again.

---

and I’d really prefer not using subtree

Yeah I'd hate to use it too, but there seems to be no better options (as of now, open to better ideas, and proven wrong), and having the limitations as mentioned above.

The number of mutation events that are generated are not different from the ones that were generated before, because debugging the number of times mutation events are generated are directly related to the number of times the load-more-items action happens.

[fregante]

I see now 👌

How about just observing the new element and removing the old one? We do that in infinite-scroll itself for the button.

[notlmn]

How about just observing the new element and removing the old one? We do that in infinite-scroll itself for the button.

I've gone ahead and tried to do this, but hit with some problems, some of them already mentioned above

• The div that we are targeting does not have any class, except for a volatile attribute
• Even if we wanted to use this volatile attribute to target that div, there are two elements inside .news that has this attribute
• Even if you used some way to find this second div, and observe it, you need to
  1. first wait for .news to mutate, then disconnect its mutation observer
  2. wait for the second div with the volatile attribute is ready, using element-ready
  3. even if you did all of this, there is no guarantee that the individual news items will be ready even when their parent element is ready 🤦‍♂️
• After everything you did until now, one day it will break, and we wouldn't even know that it did

I'm using the word volatile attribute for data-repository-hovercards-enabled here because I'm not sure we can rely on it, as there are multiple elements with the same attribute under the same parent.

I don't know of any perf issues regarding the usage of subtree, but I think it is still the easiest and reliable way of handling deep mutations, and I'm also inclined to think that it is well optimized by the browsers.

[notlmn]

Sorry for the very long explainer, and also I've made a sketch just in case for moments just like this:
https://twitter.com/ramlmn/status/1083393304878735360

[fregante]

How about 3b2772e? Not as easy as subtree but 1-5 elements listened instead of literally thousands ($$('#dashboard .news *').length)

[notlmn]

@bfred-it Yeah, good enough, but still we have to remember that if GitHub ever the DOM of dashboard (when they complete the beta dashboard), this would break again.

I'll update the PR ASAP.

[fregante]

I'll update the PR ASAP.

That's ok, I'll send that as a new PR

[notlmn]

Hey, I was supposed to work on this issue 😅!

[fregante]

Yeah by your response was that it was too complicated so I had to write it myself 😋

[notlmn]

Yeah by your response was that it was too complicated so I had to write it myself 😋

Yes the other solutions are complicated, which is why I tried to avoid them, but still subtree is performant, DOM itself is performant!

(even though I do not have any metrics 🤷‍♂️)