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

hide/show summary button sets wrong url #55

Closed
sreich opened this issue Feb 1, 2016 · 3 comments · Fixed by #58
Closed

hide/show summary button sets wrong url #55

sreich opened this issue Feb 1, 2016 · 3 comments · Fixed by #58

Comments

@sreich
Copy link

sreich commented Feb 1, 2016

clicking the + sign next to the "files changed" sets the url incorrectly (logic is backwards).

When you first load the html file, the "+" sign is visible and the files changed are hidden.

but when you click it, it sets the URL to diff.html#d2h-hide-2t9. Clicking the (now changed to) minus sign, sets the URL to diff.html#d2h-show-w29. so, this seems backwards. clicking show should set the URL to a "show" status, and visa versa.

while we're on this topic..is it possible for you to change the +/- sign to something more obvious, like "show/hide", or "show files/hide files"?

i ask because the + sign is rather obscure and i'm using this for people who aren't as tech savvy, so a tiny '+' sign would probably largely go unnoticed.

@rtfpessoa
Copy link
Owner

@sreich just did a pr in #58 what do you think?

@sreich
Copy link
Author

sreich commented Feb 8, 2016

hey, well it sounds good, (and the other issues that you worked on), but i tried using diff2html-cli 1.4.2 on windows, and it generates a page where instead of the +/- button, it shows hide.

which is great, but it's no longer a link. it's blue, but unable to click. so this actually breaks that feature (no way to expand the files changed summary)

@rtfpessoa
Copy link
Owner

Ohh, I think I missed the JS. Will look into it later.
Only tested on the diff2html side.

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

Successfully merging a pull request may close this issue.

2 participants