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

[Manager] Gray out projects in project wizard that are already attached #5862

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Vulpine05
Copy link
Contributor

Fixes #1552

Description of the Change
This PR adds a feature to the Add Project wizard that will change the text of a project to gray if the project is already added. This is accomplished by storing a string array of all projects that are available in the wizard, another string array of all projects currently attached. Each array is then canonicalized, then trimmed of "http", etc. Arrays are then compared and if any match, the text is set to gray. The ability to make the text gray was enabled by adding the property "wxLB_OWNERDRAW" to the wxListBox.

For a "simple" change, this is actually a bit more complicated. A few design notes:

  1. Adding the Ownerdraw property did more than provide the ability to change the text color. The background color of the wxListBox, wherenever there is text, also changed to grey. I discovered this was a bug and not a feature in wxWidgets, and have reported it here. It is being fixed with this pull request. I haven't tested the fix yet (I'm not sure I have the skill set to change BOINC's wxWidgets dependency). Therefore, if you were to built with this PR, it looks like this:
    image

    I had hard coded a white background before I noticed it was a bug with wxWidgets, and with that it looks like this:
    image
    Of course, that white background would be redundant once the wxWidgets PR is merged, so that hard coding is not in this PR.

  2. I have not tested this in Linux nor MacOS. I also have not reviewed the code for anything dark mode settings. I will need someone to check on those platforms to see if I broke anything. FWIW, I run Windows in dark mode.

  3. I am not an expert in accessibility, but a 4.6:1 contrast ratio looked good to me. I tried to get a 7:1 ratio, but the gray looked too close to black to me and was hard to notice the distinction.

From what I have worked on, I think this PR is ready, but I kindly request others to test this on other platforms, especially with dark mode, before merging.

Alternate Designs
Hiding the projects that are attached didn't seem like a good idea to me. If someone wanted to find out the information of the project in this wizard, they would not be able to unless they removed the project. By graying out the projects that are attached, the users is able to see which projects that are still active and they are attached to.

Release Notes
[Manager] Projects listed in Add a Project wizard that are already added are changed to grey.

The code in this commit is repeated, additionally it will be used in the same PR.
Copy link

codecov bot commented Oct 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 10.76%. Comparing base (0707685) to head (391e0ff).
Report is 407 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #5862      +/-   ##
============================================
+ Coverage     10.53%   10.76%   +0.23%     
  Complexity     1068     1068              
============================================
  Files           279      280       +1     
  Lines         35867    36494     +627     
  Branches       8409     8444      +35     
============================================
+ Hits           3779     3930     +151     
- Misses        31694    32175     +481     
+ Partials        394      389       -5     

see 124 files with indirect coverage changes

@computezrmle
Copy link
Contributor

There should be a hint explaining the meaning of the different color, like:
"To choose a project, click its name or type its URL below.
Greyed projects are already attached."

As an option or in addition something like this could be at the top of the "Project details" page:
"This project is already attached to your computer.
---"

There may also be a new entry in the "Categories" drop-down list like:
"attached"

@AenBleidd
Copy link
Member

@Vulpine05, please fix linux build

@Vulpine05
Copy link
Contributor Author

Hmm...I may have to go back to the drawing board on this one. Apparently Owner Drawn is a feature in wxWidgets for Windows build, but not for mac nor Linux. I suppose I could add precompiler code to use this feature only for Windows, but that kindof defeats the purpose of the PR if it can only work on one platform.

@Vulpine05
Copy link
Contributor Author

There should be a hint explaining the meaning of the different color, like: "To choose a project, click its name or type its URL below. Greyed projects are already attached."

As an option or in addition something like this could be at the top of the "Project details" page: "This project is already attached to your computer. ---"

When you click on the Next button a message does pop up stating that the project is added. I am assuming the user has some knowledge of what project(s) they are attached to, so I am hoping that is obvious enough. Your idea isn't bad, but in my opinion I would rather keep the window cleaner.

There may also be a new entry in the "Categories" drop-down list like: "attached"

Neat idea, but may be not as easy to add to the code. Again, just my opinion, so if there is strong support for this, I'd be willing to work on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

Hide or gray out projects already added from the BOINC Manager Add Project wizard
3 participants