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

Refactor launcher #1573

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Sajjon
Copy link

@Sajjon Sajjon commented Feb 22, 2025

Note

This is an opinionated refactoring of the Launcher feature - I completely understand if you dislike/hate
this style!

I'm new to GUI in Rust and Iced - but I've developed Redux styled applications in SwiftUI using TCA for many years, where it is quite a common pattern to split:

  • View and logic into seperate files
  • Split large view methods into many sub-view methods
  • Split large update methods into smaller sub-update methods

Which is what I've done in this PR - mostly to ask for input if it is something you as maintainers of Liana or the ICED community in large likes?

Tip

The advantages of this approach is two fold:

  1. Un-doom the pyramid of doom; less indentation in deeply nested views
  2. Code search/goto-ability improved; especially with file split one can faster get to the relevant code, and with search/goto symbol one can go straight to a method for a subview or an handle Message method.

Other changes

  • Exhaustive match on message in update => safer code

@@ -0,0 +1,36 @@
pub(super) mod prelude {
Copy link
Author

Choose a reason for hiding this comment

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

I'm no module expert - so most likely this can be improved.

.padding(100)
}

fn with_children<'a>(&'a self, content: Element<'a, Message>) -> Element<'a, Message> {
Copy link
Author

Choose a reason for hiding this comment

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

I could not elide those lifetimes, maybe someone else knows better....

)
}

fn view_no_wallet(&self) -> Column<ViewMessage> {
Copy link
Author

Choose a reason for hiding this comment

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

view_no_wallet could probably be split even more.

)
}

fn view_wallet(
Copy link
Author

Choose a reason for hiding this comment

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

view_wallet could probably be split even more.

Deleted,
}

pub(super) struct DeleteWalletModal {
Copy link
Author

Choose a reason for hiding this comment

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

DeleteWalletModal can be split out too, into children module of launcher module.

}
}

fn handle_view(&mut self, view_message: ViewMessage) -> Option<Task<Message>> {
Copy link
Author

Choose a reason for hiding this comment

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

Full exhaustive, less indented handling of ViewMessages only

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.

1 participant