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

Manually input design options outside range #6531

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

VSinerva
Copy link
Contributor

@VSinerva VSinerva commented Apr 9, 2024

This is a proposed way to close #6335

The idea is to only allow this at UX level 5, and only through "Edit settings by hand". This helps keep it the niche feature it should be, and avoids errors being generated from typos etc. in the live "Draft" view. (These came up as ideas in the discussion linked above)

All errors are still shown as before. The only difference is that at UX 5, if all errors are RangeError in options, the settings will be saved regardless. I believe this strikes a balance of not letting inexperienced (or experienced) users make mistakes during their normal workflow, while still giving experienced users increased control when explicitly asked for.

There were concerns raised in the discussion of the issue about this feature cluttering automatically generated bug reports with errors. However, I don't think that should be a deal breaker for this feature, because experienced users are already using and saving patterns with parameters out-of-range (by editing the URL). This would still be a 'hidden' niche option, so it's unlikely IMO to see much more usage than the URL-trick already does.

I'm of course open to feedback on the implementation, or on if this should be implemented at all :)

Copy link

vercel bot commented Apr 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
freesewing-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 9, 2024 10:58pm

Copy link

vercel bot commented Apr 9, 2024

@VSinerva is attempting to deploy a commit to the freesewing Team on Vercel.

A member of the Team first needs to authorize it.

@VSinerva VSinerva changed the title Manually input outside range Manually input desing options outside range Apr 9, 2024
@VSinerva VSinerva changed the title Manually input desing options outside range Manually input design options outside range Apr 9, 2024
@karlnippoldt
Copy link
Contributor

karlnippoldt commented Apr 10, 2024

I think this should be level 4 for consistency with how the user experience setting is used elsewhere. Per the documentation, there is no feature difference between level 4 and 5; the only difference is that on level 5 "“Are you sure?” confirmation guardrails removed."

For me, I'm not confident enough in FreeSewing to go without any warnings at all, but there are definitely situations where I'd want to manually input options outside the recommended range.

@HaasJona
Copy link
Contributor

HaasJona commented Apr 10, 2024

I'm not sure this is the right approach. I never use the "Edit settings by hand" tab except for copy-pasting settings. The format is weird and the units are hard to read and there's no live preview.

If we allow entering out-of-bounds values at all, I think at least this editor should also allow setting out-of-bounds values:
grafik

It's probably simple, instead of making the textfield red and disallow the input, maybe make it orange and show a warning text on experience level 4 and 5.

However I find that if people find the need to exceed the slider range, that's probably something that needs to be fixed in the design itself in the first place by extending the defined range.

That said, I think this patch makes sense and should probably be pulled in, as it would at least help somewhat.

@VSinerva VSinerva marked this pull request as draft April 10, 2024 10:11
@VSinerva
Copy link
Contributor Author

I think this should be level 4 for consistency with how the user experience setting is used elsewhere. Per the documentation, there is no feature difference between level 4 and 5; the only difference is that on level 5 "“Are you sure?” confirmation guardrails removed."

For me, I'm not confident enough in FreeSewing to go without any warnings at all, but there are definitely situations where I'd want to manually input options outside the recommended range.

Based on the documentation for UX levels, I agree. Since Edit by Hand is only available at or above level 4 anyway, I'm gonna update this PR to reflect that. Basically, edit by hand would always allow values outside the given range, but at level 4 it would ask for confirmation, while skipping that at level 5

@VSinerva
Copy link
Contributor Author

I'm not sure this is the right approach. I never use the "Edit settings by hand" tab except for copy-pasting settings. The format is weird and the units are hard to read and there's no live preview.

If we allow entering out-of-bounds values at all, I think at least this editor should also allow setting out-of-bounds values: grafik

It's probably simple, instead of making the textfield red and disallow the input, maybe make it orange and show a warning text on experience level 4 and 5.

However I find that if people find the need to exceed the slider range, that's probably something that needs to be fixed in the design itself in the first place by extending the defined range.

That said, I think this patch makes sense and should probably be pulled in, as it would at least help somewhat.

I don't necessarily disagree! The only complication is that currently the textboxes run validation+update on every change, so for level 4 that would need to be changed so it doesn't bombard the user with confirmations. That could probably be worked around by skipping the update step for out-of-range values, and only updating the pattern once the value is finalized+confirmed. I personally am not super interested in working on that rn, and I think there's value in implementing this first in a more limited scope, seeing as there were concerns raised about it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature]: enter values outside slider limit in text box
4 participants