-
Notifications
You must be signed in to change notification settings - Fork 349
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
Add custom lists to desktop GUI #5234
Conversation
DES-198 Implement custom lists GUI
Implement in accordance with the design specification in the project documents. |
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.
Reviewed 33 of 33 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved
gui/src/main/daemon-rpc.ts
line 631 at r1 (raw file):
} public async deleteCustomList(id: string): Promise<void> {
nitpick, are there no errors expected from this call?
gui/src/renderer/components/select-location/CustomListDioalogs.tsx
line 0 at r1 (raw file):
Nitpick, the name of the file is misspelled CustomListDiolags.tsx
gui/src/renderer/components/select-location/custom-list-helpers.ts
line 61 at r1 (raw file):
...list, label: list.name, list,
Is this intentianal? Spreading list and than adding the entire list?
4fc1f85
to
2bebbe4
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.
Reviewable status: 14 of 34 files reviewed, all discussions resolved (waiting on @hankolsen)
gui/src/main/daemon-rpc.ts
line 631 at r1 (raw file):
Previously, hankolsen (Hank™) wrote…
nitpick, are there no errors expected from this call?
There are. If the daemon returns an error it will be thrown from grpc and passed all the way into the renderer process where this was called from. I've added error handling in both LocationRow.tsx
and CustomListDialogs.tsx
to log errors for both this call and the updateCustomList
call.
gui/src/renderer/components/select-location/custom-list-helpers.ts
line 61 at r1 (raw file):
Previously, hankolsen (Hank™) wrote…
Is this intentianal? Spreading list and than adding the entire list?
Spreading isn't needed. Those properties weren't even used. I've cleaned up the CountrySpecification
, CitySpecification
, RelaySpecification
and CustomListSpecification
types to include only what is needed.
gui/src/renderer/components/select-location/CustomListDioalogs.tsx
line at r1 (raw file):
Previously, hankolsen (Hank™) wrote…
Nitpick, the name of the file is misspelled CustomListDiolags.tsx
Done
2bebbe4
to
9551fd1
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.
Reviewed 1 of 19 files at r2, 17 of 19 files at r3, 3 of 3 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved
This PR adds custom lists to the location selector in the desktop app. In the process I've changed how the
RelayLocation
type work. Previously it had the following signature:But that has been changed to be more intuitive and more in line with the grpc representation:
This change is