Skip to content
This repository was archived by the owner on May 11, 2025. It is now read-only.

Add bottle name validation #89

Merged
merged 10 commits into from
Jun 12, 2023
Merged

Add bottle name validation #89

merged 10 commits into from
Jun 12, 2023

Conversation

Mika412
Copy link
Contributor

@Mika412 Mika412 commented Jun 10, 2023

Currently there are no Bottle name checks during the creation and renaming. So, now it's possible to create a bottle without a name, which then uses then uses the Bottles dir as its destination, or create/rename to a name that already exists. I've added a check for the name validity, and a warning label.

Screen.Recording.2023-06-11.at.17.07.57.mov

@IsaacMarovitz
Copy link
Member

Looks good. One suggestion is that you move the warning text to the bottom left of the dialogue instead of underneath the text input so that the input boxes themselves don't get moved

Copy link
Contributor

@rhysm94 rhysm94 left a comment

Choose a reason for hiding this comment

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

I think both the approach to validation (when and how often it occurs) needs some changes, and the data type used to represent a validation state could do with some refinement too.

@Mika412 Mika412 requested review from Marsgames and rhysm94 June 11, 2023 16:35
@IsaacMarovitz IsaacMarovitz merged commit 067b5f1 into Whisky-App:main Jun 12, 2023
ohaiibuzzle pushed a commit to ohaiibuzzle/Whisky that referenced this pull request Jun 20, 2023
* Add bottle name validation

* Change view to original size

* Fix line length violation

* Replace NSLocalizedString usage

* Change warning text position

* Invert name validation logic

* Rewrite return message, as enum flags

* Update Localizable.strings

---------

Co-authored-by: Isaac Marovitz <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants