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

(GitHub diff files filter) Only «all» button seems to do anything #113

Closed
daaain opened this issue Jun 2, 2020 · 2 comments
Closed

(GitHub diff files filter) Only «all» button seems to do anything #113

daaain opened this issue Jun 2, 2020 · 2 comments
Labels

Comments

@daaain
Copy link

daaain commented Jun 2, 2020

I installed the GitHub diff files filter on Firefox with Tampermonkey.

When looking at a PR all the buttons are correctly rendered (ie file extensions and folders are picked up), but toggling anything other than the «all» button doesn't do anything.

How can I debug it?

@Mottie
Copy link
Owner

Mottie commented Jun 3, 2020

Hi @daaain!

Hmm, yeah that seems to have stopped working. I probably won't be able to work on this until the weekend.

As for debugging... it looks like GitHub removed the file anchors which were being used to target the file blocks - see https://github.com/Mottie/GitHub-userscripts/blob/master/github-diff-files-filter.user.js#L75. The userscript is using the unique SHA to target each file. It looks like it could be switched to the div id instead.

@Mottie Mottie added the bug label Jun 3, 2020
@daaain
Copy link
Author

daaain commented Jun 3, 2020

Yes, this works!

group.forEach(file_div_id => {
  const file = $(`#${file_div_id}`, files);
  if (file) {
    file.classList.toggle(`gdf-${subgroup}-hidden`, !show);
  }
});

Correction: it works for extensions, but doesn't work for folders 😿

Edit: OK, so this seems to fix the folders toggle:

if (folders[folder].length && !folder.includes("→")) {
  acc.push({
    folder,
    show: $(`.gdf-folder-filter a[data-item=${folder}]`).classList.contains(
      "selected"
    )
  });
}

It's basically throwing an error when trying to have a selector for moved files, filtering using the arrow does the trick, though I guess there might be a nicer way to do it.

@Mottie Mottie closed this as completed in 2b1c988 Jul 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants