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

[EuiFieldText] Prevent onChange event on disabled inputs #8271

Merged
merged 5 commits into from
Jan 23, 2025

Conversation

nickofthyme
Copy link
Contributor

@nickofthyme nickofthyme commented Jan 13, 2025

Summary

This alters the behavior of the EuiFieldText to prioritize disabled styling over readonly, the same as native HTML inputs.

Additionally, the disabled attribute does not prevent onChange events from firing. To counteract this behavior, we default the readonly attribute to the value of disabled unless it is explicitly set. With readonly set to true, the onChange event is prevented. Alternatively, we could conditionally call the onChange handler manually.

These changes were tested in kibana ftr in elastic/kibana#206372 with no failing tests.

Closes #8239

QA

General checklist

  • Release checklist
    • A changelog entry exists and is marked appropriately.

@nickofthyme
Copy link
Contributor Author

Looking for feedback at this point, no rush to merge.

@nickofthyme
Copy link
Contributor Author

@tkajtoch if you don't like syncing the readonly attribute with the value of disabled, I could change this to a functional prevention such as to only call the onChange when disabled is false. Lmk

Copy link
Member

@tkajtoch tkajtoch left a comment

Choose a reason for hiding this comment

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

I'm happy with this change. Could you please add a changelog item and a JSDoc comment to readOnly prop to let users know it defaults to the value of disabled now?

@nickofthyme nickofthyme requested a review from tkajtoch January 21, 2025 17:26
@tkajtoch tkajtoch marked this pull request as ready for review January 21, 2025 18:22
@tkajtoch tkajtoch requested a review from a team as a code owner January 21, 2025 18:22
Copy link
Member

@tkajtoch tkajtoch left a comment

Choose a reason for hiding this comment

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

LGTM! I'll merge it after today's EUI release

@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

@nickofthyme nickofthyme merged commit ee4999d into elastic:main Jan 23, 2025
5 checks passed
@nickofthyme nickofthyme deleted the disable-inputs branch January 23, 2025 00:50
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.

[Inputs] Styles should prioritize disabled over readOnly
4 participants