Closed Bug 456216 Opened 15 years ago Closed 15 years ago
Add attributes "first-visible" and "last-visible" to the first and last visible radio buttons of the Add-ons Manager toolbar
In bug 456214 I need a CSS selector to match the first / last visible button in the toolbar of the Add-ons manager. This toolbar is implemented as a radiogroup. :first-child and :last-child can't be used because some of these buttons might be hidden ([hidden=true] or [collapsed=true]). Most of the time the last button is in fact hidden. The solution I've implemented here just sets the attributes "first-visible" and "last-visible" on the first and last visible radio buttons of a radiogroup and updates them whenever something changes. I know this is not perfect but I didn't know another way of solving this problem.
Attachment #339598 - Flags: review?(neil)
Comment on attachment 339598 [details] [diff] [review] patch rev1 Personally I don't see what use this has outside of the addon manager which is free to set any useful attributes that you may need. I happen to know that it's a bad idea to (indirectly) call <radiogroup>'s _getRadioChildren from the <radio>'s constructor.
Component: XUL Widgets → Add-ons Manager
QA Contact: xul.widgets → extension.manager
Summary: Add attributes "first-visible" and "last-visible" to the first and last visible radio buttons of a radiogroup → Add attributes "first-visible" and "last-visible" to the first and last visible radio buttons of the Add-ons Manager toolbar
Attachment #339632 - Flags: review?(robert.bugzilla) → review?(dtownsend)
Comment on attachment 339632 [details] [diff] [review] rev2: only Add-ons manager This seems over-complicated. The first pane is always one of two and is decided at startup and never changed. The last pane is always one of 3, and you can see which straight from the booleans in updateOptionalView. Seems that a couple of if statements would do this more quickly unless I am missing something?
I didn't want it to depend on the actual button order that much, so that it doesn't have to be changed e.g. when new buttons are added. But if you prefer "hardcoding" it to the known buttons, I'll do that. You're right, it will probably be a lot simpler.
Comment on attachment 339632 [details] [diff] [review] rev2: only Add-ons manager Actually I've just come across someone doing crazy things with the UI so we'd better do it this way, however I think you can simplify it. If in the loop for setting first-visible you also cache the last visible tab you can do away with the second loop I believe.
Attachment #339632 - Flags: review?(dtownsend) → review-
Comment on attachment 340928 [details] [diff] [review] rev3: better loop Looks good. Can you just rename the method to updateVisibilityFlags though. We aren't changing visibility here so this makes more sense I think.
Attachment #340928 - Flags: review?(dtownsend) → review+
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b1
You need to log in before you can comment on or make changes to this bug.