After a lot of questions I've just got a tech-debt bug that needs filing, a question, and some wording suggestions. I also have some questions I'd like the UI reviewer to answer.
The tech-debt bug: you copy a big chunk of code into activate_collapsible_div. We talked about this on IRC and you explained that you'd tried refactoring, but changing the code to work on <div> instead of <fieldset> made it a huge pain. We agreed that you'd file a tech-debt bug and put an XXX in the code.
The question: I don't know if it makes sense to use waitForPageLoad when the page has already loaded. Is there a better way to wait for some Javascript to run? If so, please change this.
Wording suggestions:
1. s/"safe use this"/"safely use this"
2. In your explanatory text, you need to distinguish between the configuration links and the collapsible element that *contains* the configuration links.
2a. # When the Show link is clicked when it's open, it closes it.
=>
# When the collapsable element is open, clicking the Show link will close it.
2b. "Additionally, on a not fully configured project, it starts by showing
the links, and can be closed."
=>
"On a not fully configured project, the collapsable element starts off
open and displaying the configuration links, but it can be closed."
Questions that I defer to the UI reviewer:
1. Do you need that <br /> on line 270? If so, maybe it should also be conditional on view/configuration_links? A <br /> with nothing after it doesn't look good.
2. "Configuration links" is a little vague and passive. Maybe "Configure this project" would be better wording?
After a lot of questions I've just got a tech-debt bug that needs filing, a question, and some wording suggestions. I also have some questions I'd like the UI reviewer to answer.
The tech-debt bug: you copy a big chunk of code into activate_ collapsible_ div. We talked about this on IRC and you explained that you'd tried refactoring, but changing the code to work on <div> instead of <fieldset> made it a huge pain. We agreed that you'd file a tech-debt bug and put an XXX in the code.
The question: I don't know if it makes sense to use waitForPageLoad when the page has already loaded. Is there a better way to wait for some Javascript to run? If so, please change this.
Wording suggestions:
1. s/"safe use this"/"safely use this"
2. In your explanatory text, you need to distinguish between the configuration links and the collapsible element that *contains* the configuration links.
2a. # When the Show link is clicked when it's open, it closes it.
=>
# When the collapsable element is open, clicking the Show link will close it.
2b. "Additionally, on a not fully configured project, it starts by showing
the links, and can be closed."
=>
"On a not fully configured project, the collapsable element starts off
open and displaying the configuration links, but it can be closed."
Questions that I defer to the UI reviewer:
1. Do you need that <br /> on line 270? If so, maybe it should also be conditional on view/configurat ion_links? A <br /> with nothing after it doesn't look good.
2. "Configuration links" is a little vague and passive. Maybe "Configure this project" would be better wording?