Skip to content

Conversation

@jefersonla
Copy link

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.

@dderevjanik
Copy link
Owner

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

@jefersonla
Copy link
Author

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

@s-weigand
Copy link
Contributor

s-weigand commented Apr 5, 2021

@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.:

export function initGithub() {
// Update on fragment update
observe(QUERY_FILE_TABLE_ITEMS, {
add(rowEl) {
showRepoTreeIcons(rowEl);
},
});
update();
document.addEventListener('pjax:end', update); // Update on page change
}

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 🥂

@dderevjanik
Copy link
Owner

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

@maicol07
Copy link

maicol07 commented Nov 3, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants