Conversation
…ble on validation errors
|
Summary of ChangesHello @varun-srinivasa, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an inconsistency in the behavior of Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where readonly date and time inputs become interactive when a validation error is displayed. The changes prevent the dropdown from opening on focus and remove the dropdown trigger when the component is in a readonly or disabled state. The implementation is correct and effectively solves the problem. I've added a couple of suggestions to improve code maintainability by encapsulating the repeated condition into a getter.
| if (!this.readonly && !this.disabled) { | ||
| this.openDropdown(); | ||
| } |
There was a problem hiding this comment.
The condition !this.readonly && !this.disabled is now used in two places in this file. To improve code clarity and maintainability, you could introduce a private getter for this logic.
For example:
private get isInteractive() {
return !this.readonly && !this.disabled;
}You could then use this.isInteractive here and for the trigger prop of the ix-dropdown component (lines 555-557). This would make the code more readable and easier to maintain if the logic for determining interactivity changes in the future.
| if (!this.readonly && !this.disabled) { | ||
| this.openDropdown(); | ||
| } |
There was a problem hiding this comment.
Similar to the date-input component, the condition !this.readonly && !this.disabled is used twice here. To avoid duplication and improve readability, consider extracting this logic into a private getter.
For example:
private get isInteractive() {
return !this.readonly && !this.disabled;
}This getter can then be used here and for the trigger prop of the ix-dropdown component (lines 569-571).
|
1 participant
💡 What is the current behavior?
When working with read-only forms, I noticed something unexpected happens during validation. If there is an error in any input value, the expected behavior is that the input remains non-interactive however when the error appears the input becomes selectable/interactive. only in date and time input
GitHub Issue Number: IX-3909
🆕 What is the new behavior?
Now even in readonly form the date and time input components are not interactive
🏁 Checklist
A pull request can only be merged if all of these conditions are met (where applicable):
pnpm test)pnpm lint)pnpm build, changes pushed)👨💻 Help & support