-
Notifications
You must be signed in to change notification settings - Fork 320
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
Don't overwrite the HREFs for the Download IntelliJ/SBT buttons #1294
Conversation
This should fix scala#1285 for good. In scala#1286, it was unbeknownst to me that there was JS that ended up editing the HREFs of the IntelliJ and SBT links, and so the code that was initially broken there remained broken.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
That being said, it seems that #download-step-one
is not present on the HTML page, do you know when it was removed?
Huh, I have no clue. Should I remove this entire |
Should I merge this now in order to get the basic bugfix in, and then we can continue discussing further changes? (It's possible I've misunderstood the situation.) |
Ok to merge this now. |
thanks Aly! (and Julien!) |
What should we do about |
If you intend to tackle it yourself, you can just PR a fix directly (or send a "should we even merge this?" draft PR, if it needs discussion). But opening an issue and hoping someone else eventually deals with it is fine too. |
I am not sure. The goal was to show specialized instructions according to the OS of the user. It would be helpful to investigate whether these specialized instruction still make sense or not, and to re-introduce them, if it makes sense. I don’t have the bandwidth to do that myself, but if you are willing to help that would be greatly appreciated! |
This should fix #1285 for good. In #1286, it was unbeknownst to me that there was JS that ended up editing the HREFs of the IntelliJ and SBT links, and so the code that was initially broken there remained broken.
I've tested this by making the relevant changes in the Chrome Debugger and verifying that the HREFs aren't overwritten.