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

fix: readonly number fields should not show controls #829

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

kerlon5
Copy link
Contributor

@kerlon5 kerlon5 commented Oct 2, 2023

Closes #810

@kerlon5
Copy link
Contributor Author

kerlon5 commented Oct 2, 2023

Closes #810

Copy link
Contributor

@pinussilvestrus pinussilvestrus left a comment

Choose a reason for hiding this comment

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

@kerlon5 thanks a lot for your contribution!

What I'm wondering is that the number field is not distinguishable now with text fields in the read-only state. Also in in read-only, it might be interesting what data type this field has, although I can't edit it.

image

@bpmn-io/hto-dev @volodymyr-melnykc what do you think?

@volodymyr-melnykc
Copy link

In the read-only mode, we don't need to show controls as the user can't interact with it, so I agree with the fix.

If an input field has a "prefix" or "suffix" defined, we should still display it when in read-only. I believe that this remains the case (based on code updates).

@kerlon5 @pinussilvestrus

Copy link
Contributor

@pinussilvestrus pinussilvestrus left a comment

Choose a reason for hiding this comment

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

Then let's go for it 👍

Also to confirm the adorners work as expected
image

Thanks @kerlon5 again 🚀

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.

3 participants