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

Diff filter does not work #2613

Closed
FreHu opened this issue Apr 1, 2019 · 2 comments · Fixed by #2662
Closed

Diff filter does not work #2613

FreHu opened this issue Apr 1, 2019 · 2 comments · Fixed by #2662

Comments

@FreHu
Copy link
Collaborator

FreHu commented Apr 1, 2019

I noticed this some time ago, then forgot to report it and then I wanted to check if the new version didn't fix it somehow.

The filter doesn't respond to clicks at all (diff, patch page):
image

Reproducible in 1.86 and 1.86.2. Very likely to be broken in 1.86.1 as well, I just haven't tried that version at all.

Possibly related:
image

I'm not able to track down the source of the error. Could be some weird character issue again, although I get it even on an fresh repository (on every page, even repo settings).

I managed to find this line, which is missing quotes around the id:
https://github.com/larshp/abapGit/blob/604c68200ed6a038229756f179fb86b9ccc9cc39/src/ui/zcl_abapgit_gui_page_diff.clas.abap#L681

I can submit a PR for the quote fix, maybe it doesn't change anything, but it is incorrect.
I get the error even with the fixed quotes though.

@FreHu
Copy link
Collaborator Author

FreHu commented May 7, 2019

@sbcgua
Copy link
Collaborator

sbcgua commented May 7, 2019

good find. I can't remember why the octicon is checked there though it seem to be my code. Probably some extra assert that it is the item list. The quick fix would be to change to contains("icon") in js

Do you prefer to PR or I can do it ?

@FreHu FreHu mentioned this issue May 7, 2019
larshp pushed a commit that referenced this issue May 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants