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(scheduler): invert external/internal toggle #257

Merged
merged 12 commits into from
May 12, 2022

Conversation

dergigi
Copy link
Contributor

@dergigi dergigi commented May 11, 2022

This PR inverts the external address toggle and adjusts the behavior of the scheduler to "internal" by default.

Also adjusts and updates copy, fixing #248

Screenshot 2022-05-11 at 15 11 07

Screenshot 2022-05-11 at 15 11 18

@dergigi dergigi requested a review from a user May 11, 2022 13:16
@dergigi dergigi self-assigned this May 11, 2022
ghost
ghost previously requested changes May 12, 2022
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

That's a good change! Much clearer than before. 👏 👍

One issue though: When the screen initially loads, the internal destination addresses won't be set and the button "Start" button will be disabled. That's because the internal addresses will only be set once the toggle is actively un-toggled. Only after toggling the button back and forth will the internal addresses be set and the button be activated.

With the new logic, we'll need to provide internal destination addresses as default values for the address fields.

Screen.Recording.2022-05-12.at.10.11.08.mov

@ghost ghost force-pushed the invert-schedule-toggle branch from b6536b2 to e2f1d59 Compare May 12, 2022 09:19
@ghost
Copy link

ghost commented May 12, 2022

Pushed a fix that should work (I hope). TL;DR: Waits with rendering the form unil the wallet is loaded. Once the wallet is loaded, sets the form's initial values to 3 internal addresses.

@theborakompanioni Can you have a quick look please? 🙏 Don't want to approve my own code. 😅

@ghost ghost dismissed their stale review May 12, 2022 09:22

Issue should be fixed

@ghost ghost requested a review from theborakompanioni May 12, 2022 09:22
@dergigi dergigi added the UI/UX Issue related to cosmetics, design, or user experience label May 12, 2022
@dergigi
Copy link
Contributor Author

dergigi commented May 12, 2022

When the screen initially loads, the internal destination addresses won't be set

Ah, damn. Thanks for catching and fixing that! 🙏

Copy link
Collaborator

@theborakompanioni theborakompanioni left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@theborakompanioni theborakompanioni merged commit 030f51a into master May 12, 2022
@theborakompanioni theborakompanioni deleted the invert-schedule-toggle branch May 12, 2022 12:20
@ghost ghost mentioned this pull request May 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI/UX Issue related to cosmetics, design, or user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants