Skip to content

Add ability to save, edit and load custom practice states#348

Draft
LukeSaward wants to merge 5 commits intomasterfrom
122-custom-practice-states
Draft

Add ability to save, edit and load custom practice states#348
LukeSaward wants to merge 5 commits intomasterfrom
122-custom-practice-states

Conversation

@LukeSaward
Copy link
Collaborator

@LukeSaward LukeSaward commented Aug 1, 2024

Resolves #122

Copy link
Owner

@oberien oberien left a comment

Choose a reason for hiding this comment

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

To have the parsing in the UI element itself, the UI element needs a way to display error messages. In general we seem to be using the pattern quite a lot where we modify the label of an input when parsing the value either onchange or onclick. It might make sense to add that as option to the existing or as new UI-Elements to reduce lots of boilerplate in the future.

I see 3 different ways to achieve this:

  • Have the event functions return a string which is added to the label.
  • Pass the label to the function and have the label have its own suffix field and function, which gets reset before calling the event function and concatenated afterwards.
  • The UI Element could store the old value and reset it every change.

The latter may be the cleanest as it doesn't require a Label to be { text: string, suffix: string }, which might not make much sense. Though it also feels a lot like magic and be unintuitive if the UI Element automatically resets the label all the time.

That change could also be done in a different PR as this one is already quite large.

@LukeSaward LukeSaward force-pushed the 122-custom-practice-states branch from d7856e3 to cc383dd Compare July 23, 2025 00:37
@LukeSaward LukeSaward force-pushed the 122-custom-practice-states branch from a1b965a to 6a2ff73 Compare August 2, 2025 11:09
@LukeSaward LukeSaward force-pushed the 122-custom-practice-states branch from b619db4 to 5405951 Compare August 3, 2025 20:04
if key.to_small() == KEY_BACKSPACE.to_small() {
self.input = self.input.slice(0, -1);
} else if key.to_small() == KEY_V.to_small() && (LCTRL_PRESSED || RCTRL_PRESSED) {
self.input = f"{self.input}{Tas::get_clipboard()}";
Copy link
Owner

Choose a reason for hiding this comment

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

You never check if the clipboard is valid. This way the user could paste non-number characters and Refunct will crash the next time onclick is called.

Option::None => (),
}
}
match character.parse_int() {
Copy link
Owner

Choose a reason for hiding this comment

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

This should probably use is_digit.

@LukeSaward LukeSaward force-pushed the 122-custom-practice-states branch from 5405951 to 5ef784e Compare August 29, 2025 19:07
@LukeSaward LukeSaward marked this pull request as draft August 29, 2025 19:30
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.

Custom Practice States

2 participants