-
Notifications
You must be signed in to change notification settings - Fork 32
feat: πΈ preferred clients section on settings page #3034
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
feat: πΈ preferred clients section on settings page #3034
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
550654c to
1cbed00
Compare
1cbed00 to
d129e23
Compare
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.
This looks great! Would this work already with a test desktop client build? I can give that a try tomorrow if so.
My feedback is:
- another test (or test assertion in the existing test) to check that the data is responding from the
changeevent - try and assign these known set of values to constants that are exported from the best source module (maybe
ui/desktop/app/services/rdp.js)
ui/desktop/tests/integration/components/settings-card/preferred-clients-test.js
Show resolved
Hide resolved
Yes, we can test with local DC build. |
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.
I think the logic around getting the clients could be less coupled in how it's set up. Also if the methods have several conditions in a service and they're not exercised by the rendering or acceptance tests it would be good to have unit tests around them.
I think there are some cases here that might need a few acceptance since they rely on certain state being set in the beforeModel hook and it would be good to ensure that the data coming from ipc is set up on the rdp service and then seen on the settings. That could be done up in a follow-up PR though.
ui/desktop/tests/integration/components/settings-card/preferred-clients-test.js
Outdated
Show resolved
Hide resolved
ui/desktop/app/components/settings-card/preferred-clients/index.hbs
Outdated
Show resolved
Hide resolved
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.
π Looks good! Thanks for all the work on this
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.
looks great! π
104a4e6
9f0cf22 to
104a4e6
Compare
* feat: πΈ preferred clients section on settings page * chore: π€ update mock ipc with new handlers * refactor: π‘ moved windows rdp protocol to const * refactor: π‘ moved rdp calls to app route * test: π fixed failings tests * refactor: π‘ addressed comments * fix: π fixed failing tests * refactor: π‘ add mock rdp service calls to tests * refactor: π‘ addressed comments * test: π add additional tests * refactor: π‘ fixed test failure
* feat: πΈ preferred clients section on settings page * chore: π€ update mock ipc with new handlers * refactor: π‘ moved windows rdp protocol to const * refactor: π‘ moved rdp calls to app route * test: π fixed failings tests * refactor: π‘ addressed comments * fix: π fixed failing tests * refactor: π‘ add mock rdp service calls to tests * refactor: π‘ addressed comments * test: π add additional tests * refactor: π‘ fixed test failure
Description
Adds a new Preferred Clients section on Settings Page.
ποΈ Jira ticket
Screenshots (if appropriate)
Mac:
Screen.Recording.2025-10-27.at.12.35.57.PM.mov
None detected:
Screen.Recording.2025-10-28.at.5.24.31.PM.mov
Windows:
Screen.Recording.2025-10-27.at.1.06.55.PM.mov
None detected:
Screen.Recording.2025-10-28.at.6.05.18.PM.mov
How to Test
Checklist
a11y-testslabel to run a11y audit tests if neededPCI review checklist
Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.