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

feat(launchpad): connection mode and fixes #2076

Merged
merged 24 commits into from
Sep 3, 2024

Conversation

mazzi
Copy link
Contributor

@mazzi mazzi commented Aug 29, 2024

Description

  • We create a new option under Options screen called Connection Mode, were the user can switch between Automatic, Home Network, UPnP, Custom Ports.
  • We fixed a bug and improve navigability.
  • We introduce error popups.
  • We unified screen styles.

A technical overview

NAT detection

Now we use the latest nat-detection crate on crates.io. Thanks to a bug fixed on sn-releases.

Error Handling

A new module was created to show error messages on popups. Can be plugged on any component. Documentation on the file.

Port Popup

It is a separate popup, in opposition to the established strategy in Drive Selection and Beta Programme, were follow up popups are self contained on the same file. The reason for this is that we can invoke the Port Selection popup directly from the Options screen.

Node mgmt

A couple of refactors there were we created helper functions to be able to understand better the algorithm.

Bug fixes

The application will run despite app_config.json doesn't have certain keys related to mountpoints.

Dependencies

Updated sn-releases to 0.2.8.

Testing

Desktop testing was made. Mostly on macOS. We will need to test more on Windows/Linux.

Screenshots

Screenshot 2024-08-29 at 16 57 33 Screenshot 2024-08-29 at 16 57 43

Screenshot 2024-08-29 at 16 57 52 Screenshot 2024-08-29 at 16 58 03 |

@mazzi mazzi force-pushed the feat_connection_mode branch from 80599af to 6a9c131 Compare August 29, 2024 14:54
retry_count = 0; // Reset retry count on success
}
Err(err) => {
//TODO: We should use concrete error types here instead of string matching (sn_node_manager)

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
Copy link
Contributor

@joshuef joshuef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on mac, code looks good! Nice one @mazzi

Although I dont understand how to get to the port popup there 🤔

@mazzi
Copy link
Contributor Author

mazzi commented Aug 30, 2024

Tested on mac, code looks good! Nice one @mazzi

Thanks!

Although I dont understand how to get to the port popup there 🤔

When Custom Ports is selected, the follow up screen should be Custom Ports popup. Anytime with Custom Ports selected as Connection mode, you can go to Options screen, hit Ctrl+P and you should be able to edit your custom ports. In the other modes of course, the Port selection popup is disabled.

joshuef
joshuef previously requested changes Sep 2, 2024
Copy link
Contributor

@joshuef joshuef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, with the above, I cannot save my custom port range. Hitting enter does nothing for me 🤔

@mazzi
Copy link
Contributor Author

mazzi commented Sep 2, 2024

Aha, with the above, I cannot save my custom port range. Hitting enter does nothing for me 🤔

Most likely your port range is out of a valid one.
We only enable Enter when the range is between PORT_MIN (1024) and PORT_MAX (65535)
Perhaps we will need to provide more feedback to the user...

@mazzi mazzi force-pushed the feat_connection_mode branch from 6485c94 to 49d7248 Compare September 2, 2024 11:42
@mazzi
Copy link
Contributor Author

mazzi commented Sep 2, 2024

Aha, with the above, I cannot save my custom port range. Hitting enter does nothing for me 🤔

I think that your claim is valid as you didn't know this screen beforehand. We added feedback to the user in case that the port range is invalid 👍

@mazzi mazzi requested a review from joshuef September 2, 2024 11:45
@mazzi mazzi dismissed joshuef’s stale review September 2, 2024 12:35

Implemented feedback to the user to understand why "enter doesn nothing".

@mazzi mazzi force-pushed the feat_connection_mode branch from 49d7248 to c3b3d56 Compare September 2, 2024 16:23
@joshuef joshuef added this pull request to the merge queue Sep 3, 2024
Merged via the queue into maidsafe:main with commit 8e11156 Sep 3, 2024
40 checks passed
@mazzi mazzi deleted the feat_connection_mode branch September 3, 2024 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants