GitHub Code View Support

[csandman]

This PR closes #55

In order to support GitHub's new code view (which is currently only in preview mode and is still potentially subject to change), there was more involved in the icon replacement than simply adding new selectors, which I already mentioned in this comment: https://github.com/Claudiohbsantos/github-material-icons-extension/issues/55#issuecomment-1447100048.

The first change I made was somewhat unrelated, but I felt it was a good choice for consistency. I moved the style injected into the stylesheet on the Azure website into the normal injected stylesheet, which is used for the icon sizing. I think this makes more sense than checking if the style has been added over and over. I did, however, also change the name of the "hide pseudo" class to be more specific to ensure this style doesn't conflict with any other git providers' built in class names.

Next, I added the new selectors required as per usual. I ended up splitting them onto multiple lines, as they were getting really long and difficult to read. This also let me realize that two of the selectors were redundant (probably my fault haha):

.octicon-file, a.tree-browser-result > svg.octicon.octicon-file

so I removed the second of these two.

And then came some complications. Like I mention in the comment above, the nature of the tree view used by the new code view is different from the PR code view. For these, if you replace the icon with the custom icon, React (what GitHub is made with) will break every time you navigate to a new sub-folder, or expand/collapse one of the folders in the sidebar. So I used a similar approach to how I replaced the icons in Azure. Instead of straight up replacing them, I added them as a sibling to the old icon and added a class to the old icon to hide it.

This worked quite well for the most part, except for the open/closed folder icons on the sidebar. When you click on the caret to open them, the selector observer is not triggered, so the new icon never receives the class it needs to hide the old icon.

---

The way I managed to solve this was simply by adding a global style to hide that specific icon type:

.PRIVATE_TreeView-directory-icon svg {
  display: none !important;
}

The problem with this approach is that on first load, the icons are entirely hidden, which causes a layout shift of the directory names when the icons are replaced... and while writing this, I actually realized a more consistent approach would just be to add a sibling selector from the custom icons. That way, the old icon is only hidden once it is replaced by the new one, and we don't need a custom class on the old icon in order to hide it:

img[data-material-icons-extension='icon'] + svg {
  display: none !important;
}

I would have made the selector more specific, like svg.octicon, but for some reason the icons inside the main body of the code view don't have the same classes as the rest. I don't believe there are any other cases where an SVG immediately follows the custom icons in any of the other Git providers, though, so I think this should be fine.

I also tested this strategy of adding the new icon as a sibling across all the different GitHub pages where this extension functions, and it appears that it works well on all of them.

---

The one other issue I ran into is the Parent Directory icon isn't replaced in the same way as the others.

It does not follow the same class naming conventions that the rest of the rows in the table do, so it wasn't the simplest thing to target. I did manage to find some selectors that worked, but they feel way too general. Here is what I tried that worked:

  selectors: {
    row: `.js-navigation-container[role=grid] > .js-navigation-item,
      file-tree .ActionList-content,
      a.tree-browser-result,
      .PRIVATE_TreeView-item-content,
      table[aria-labelledby="folders-and-files"] tr`, // This is the closest I could get to a specific selector
    filename: `div[role="rowheader"] > span,
      .ActionList-item-label,
      a.tree-browser-result > marked-text,
      .PRIVATE_TreeView-item-content > .PRIVATE_TreeView-item-content-text,
      tr h3`, // Very general
    icon: `.octicon-file,
      .octicon-file-directory-fill,
      .octicon-file-directory-open-fill,
      .octicon-file-submodule,
      tr svg`, // Also very general
  },
  getIsLightTheme: () => document.querySelector('html').getAttribute('data-color-mode') === 'light',
  getIsDirectory: ({ icon, row }) =>
    icon.getAttribute('aria-label') === 'Directory' ||
    icon.classList.contains('octicon-file-directory-fill') ||
    icon.classList.contains('octicon-file-directory-open-fill') ||
    icon.classList.contains('icon-directory') ||
    row.innerText.includes('directory'), // Used for the "preious directory" link in the code view

I can switch to this approach if you'd like, otherwise I can leave it out! Let me know if you have any thoughts on the way I implemented things as well.

[Claudiohbsantos]

Thanks for getting this started and the in depth description! I only just gleamed through the description so I'll have to come back for the actual code later once I play around with the new view to understand the differences better, but just wanted to comment on some top level thoughts before you get too far:

• Since this feature is still in preview I think we have 2 possible approaches to the problem:
  1. Implement changes that work for both the old code view and the new one AND do not change the behavior of the existing standard view. Since most users will likely remain in the standard code view I want to make sure we don't break that experience.
  2. Implement the new code view as an "opt-in" feature that can be toggled via the extension options. This should only be the case if approach *1 is impossible for some reason because it probably introduces a lot of code we now also need to maintain.

I'll try to check back on this PR properly next week when I should have more free time again. Thanks!

[csandman]

i. Implement changes that work for both the old code view and the new one AND do not change the behavior of the existing standard view. Since most users will likely remain in the standard code view I want to make sure we don't break that experience.

I completely get this perspective! Overall, I think this solution shouldn't cause any problems with the existing code views, even though it does change how they function. But this should be tested further to ensure that it's true, because you're right, it would suck if it didn't work for people who aren't even using the new view.

Take your time looking at it, I just figured I'd take a shot at it for fun, so I don't really care when it's merged! I'm also going to be running this version for the foreseeable future to see if it breaks at any point.

[Claudiohbsantos]

@csandman thanks for all the great work here. "Next week" became "in a couple months" but I finally managed to get some time to clean up the cobwebs and test this out for both the beta and the legacy views. This is looking great! I still see some light "blinking" on load on the new code view but I'll merge this as is since this is an infinite improvement over not working at all, so we can tackle that later if it persists.

[csandman]

@csandman thanks for all the great work here. "Next week" became "in a couple months" but I finally managed to get some time to clean up the cobwebs and test this out for both the beta and the legacy views. This is looking great! I still see some light "blinking" on load on the new code view but I'll merge this as is since this is an infinite improvement over not working at all, so we can tackle that later if it persists.

Yeah I still see the blinking as well, especially if the icons aren't cached I think? either way, I think that might just be due to the different way they render the new code view. I think, because it's a much more dynamic view, where new components are loaded in as you navigate, there might be more of a delay in when the plugin sees the icons? Not sure, but this was the closest I could get it, and I still think it looks pretty good, so I'm glad you merged!

Also, I'm not sure if you took a look at the bottom part of my PR description, but I did figure out that roundabout method for getting the "parent folder" icons to change. Do you have any opinions on that part?