github-vscode-icons icon indicating copy to clipboard operation
github-vscode-icons copied to clipboard

Repair select-dom import and the issue with icon display on GitLab

Open jefersonla opened this issue 4 years ago • 5 comments

This pull-request updates the import style of the 'select-dom' module, which is currently not working because version 7.1.0 of 'select-dom' added TypeScript bindings and change the import style (ES module default). Without 'select-dom' no icons are showed on GitLab and SourceForge websites.

But even after repair 'select-dom' import, no icons are updated in GitLab, for two reasons:

  1. First, GitLab has changed the "dom layout" of tree-items;
  2. Second GitLab list files asynchronous sometimes.

In order to repair this problem, I've created a MutationObserver that apply vscode-icons for items added after the plugin initialization.

jefersonla avatar Mar 28 '21 23:03 jefersonla

@jefersonla Thank you for your contribution. Everything works fine 👍 ! Will try to release this fix ASAP

dderevjanik avatar Apr 04 '21 13:04 dderevjanik

Thanks @dderevjanik, but i think there's some problem with the imports and Jest, have you already experienced that?

jefersonla avatar Apr 05 '21 13:04 jefersonla

@jefersonla The support for ES module in jest is atm only available in the beta releases, I made an updated tests fix PR #36 to support them. I checked out your branch and tested that the tests pass. Since typescript handles things differently than plain node are you sure that changing the import is needed and it wasn't an issue with the async loading of the icons? I mean tests pass, so that's an indication that select-dom was compiled successfully.

Also, since selector-observer is already a dependency you can just use it and don't need to write a MutationObserver from scratch. E.g.: https://github.com/dderevjanik/github-vscode-icons/blob/0e87e4868e1357a54a20c44d1fddacb5e4f3a0bc/packages/content/pages/GitHub.ts#L115-L124

To check for future changes in the page layout of GitLab it would also be nice if you could add the selector for the icons to the test.

And two more things:

  • If you install the dependencies with npm ci you won't change the package-lock.json (no merge conflicts) and install the packages the same way as the CI
  • If make PRs from a feature branch instead of the default branch (here master), maintainers of a repo can push to it

Cheers 🥂

s-weigand avatar Apr 05 '21 14:04 s-weigand

@jefersonla sorry for late response, but could you please resolve conflics?

dderevjanik avatar Apr 13 '21 21:04 dderevjanik

Will this be merged? Currently, gitlab has no icons...

maicol07 avatar Nov 03 '21 18:11 maicol07