-
-
Notifications
You must be signed in to change notification settings - Fork 7k
Add titles to rows in the manager tables #9262
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
Conversation
Hi Joe, while I love the patch, visually it makes things a bit different from what it used to be |
The bigger font is nice for those who are low vision, but the repeat of the text on two lines seems awkward. Is there a nightly build I can run it on to hear what it's like in a screen reader? And yes, I agree that using the isAccessible() flag when possible is a good option. |
@ArduinoBot build this please |
The build should be coming shortly 😉 |
✅ Build completed. Please test this code using one of the following: ⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-9262-BUILD-896-linux32.tar.xz ℹ️ The |
Thanks @facchinm Unfortunately the PR has the same problem as before, as Joe mentioned. It doesn't work on MacOS yet, so I can't test until I'm back on a PC. It behaves the same as previously: freezes up at the filter box. |
I had at one point taken the title out of the actual cell and only left it in the TitleBorder. But then it didn't read well in the screen reader, so I put it back into the cell.
Maybe taking it out of the cell, but putting it into the accessible text is the way to go? I kinda dislike this idea because I'm not a fan of the accessible text not matching the on screen text. But it might be a good way to go anyway?
On Monday, September 30, 2019, 04:15:27 AM EDT, Martino Facchin <notifications@github.com> wrote:
Hi Joe, while I love the patch, visually it makes things a bit different from what it used to be
I kinda like the new look but maybe just exposing under isAccessible() flag could be better right now. What do you think? @tigoe
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
If it's a choice between where to put the title, I actually prefer the larger type of the title in the bounding box, personally. I'd leave it there and cut the title that's just below it. But I recognize that there may be factors I'm not considering. |
I have made a new push to my repository. I remove the name from the cell, leaving it only in the title. This is pretty good, but not perfect. As I enter the cell it will read the title, then the contents of the cell string. Even at awkward in one use case, I still think this is the better way to go. |
Is there an update on this? More that I need to do? |
I need to retest it in the next few days, sorry for the radio silence but we are a bit overwhelmed in this period 🙂 |
OK sounds good to me! |
Looks perfect now, merging. Thanks again! |
In commit 93581b0 (Set foreground color in library/board manager), the foreground color was set in addition to the background color, to make sure that the library and board manager would remain readable even with a non-standard color scheme (e.g. a dark theme). When that commit was created, this worked properly. However, between creating that commit and merging it as part of arduino#9272, the title rendering was changed from being part of the description (which had its color set up properly) to being part of the title border (which used default colors) in arduino#9262. This commit fixes this again by applying the foreground color also to the TitledBorder component.
In commit 93581b0 (Set foreground color in library/board manager), the foreground color was set in addition to the background color, to make sure that the library and board manager would remain readable even with a non-standard color scheme (e.g. a dark theme). When that commit was created, this worked properly. However, between creating that commit and merging it as part of arduino#9272, the title rendering was changed from being part of the description (which had its color set up properly) to being part of the title border (which used default colors) in arduino#9262. This commit fixes this again by applying the foreground color also to the TitledBorder component. In ContributedPlatformTableCellJPanel this also moves the creation of the TitledBorder into the constructor, and for ContributedLibraryTableCellJPanel upwards in the constructor (where it is run unconditionally), so the property can be final.
When arrowing through the Library Manager table and the Board Manager table while using a screen reader, the rows are not read.
By adding a TitleBorder the rows of the table get read.
This currently does not work in MacOS - JDK 9+ is required for it to work on MacOS. As such, I hope to soon write a replace to these managers for when accessibility features setting is turned on so that the current tables are avoided. However this may take some time.
All Submissions:
New Feature Submissions:
Changes to Core Features: