search

material-extensions / material-icons-browser-extension

Browser Addon that enhances file browsers of version controls with material icons.
MIT License
499 stars 38 forks source link

Extension not supported in new code view mode #55

Closed Repiteo closed 1 year ago

Repiteo commented 1 year ago

The recently implemented new code view completely breaks the extension's functionality

Currently the mode is in feature-preview, so probably not a top-priority

How this repository currently appears with the feature enabled

shivapoudel commented 1 year ago

@Claudiohbsantos Yes, it's true. I just wanted to create an issue ticket but it exists :D

image

csandman commented 1 year ago

@Claudiohbsantos I can take a stab at this one if you like. I already tried it out a bit because I was curious, and I've noticed it has the same issue Azure did, where directly replacing the icon element breaks the page. This happens because React is trying to replace the previous svg element with new ones as you navigate through the directories. Which results in this being displayed:

image

and this error in the console:

DOMException: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.

I believe I have a solution for it, similar to how I solved the problem with Azure. Instead of actually replacing the icon, I tried appending the new icon as a sibling of the old one and adding styles to hide the original icon. This appears to work pretty well, but I need to figure out a couple edge cases and make sure nothing else is broken by these changes.

As a side note, I did not have to add the custom mutation observer in order to make this work, so this should be simpler than the Azure provider at least.

Claudiohbsantos commented 1 year ago

Oh no! The dreaded day when github changes their DOM and breaks everything has finally arrived 😱 To be honest I'm very surprised it has taken this long for this to come up hahaha

Thanks @csandman I'll take a look at the new github code view and will follow up on your PR once I understand the problem a bit better.

so1ve commented 1 year ago

https://github.com/homerchen19/github-file-icons/pull/134

This might help

shivapoudel commented 1 year ago

@Claudiohbsantos Any updates on this so far?

kidonng commented 1 year ago

I took the liberty of applying #56 (and #59) to current HEAD (ff97e509806357721fb89dc83593ad1a05f5426a) and build a patched version of the extension. You are welcome to sideload it before a proper fix is released.

All credits go to @csandman.

By using my build you trust me even though I'm a random person on the Internet. If you don't you should build it yourself, it's really simple.

github-material-icons-chrome-extension.zip

github-material-icons-edge-extension.zip

github-material-icons-firefox-extension.zip

Side note: if you are also a Refined GitHub user you should disable the quick-file-edit feature as it's currently incompatible.

Claudiohbsantos commented 1 year ago

Thanks for making a build available @kidonng ! I just managed to work my way through the PRs and this has now been merged. I'll kick off a release in a sec, it should be up in the stores asap