Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

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.

None yet

4 participants