-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Implemented a one-click toggle for the custom time input bar #4956
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ This pull request was sent to the PullRequest network for review. Expert reviewers are now being matched to your request based on the code's requirements. Stay tuned!
What to expect from this code review:
- Comments posted to any areas of potential concern or improvement.
- Detailed feedback or actions needed to resolve issues that are found.
- Turnaround times vary, but we aim to be swift.
@ezParth you can click here to see the review status or cancel the code review job.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PullRequest Breakdown
Reviewable lines of change
+ 20
- 10
73% JavaScript
27% TSX
Type of change
Minor Update - These changes appear to be a minor update to existing functionality and features.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the code it seems like it would fix the problem so I'm giving this a 👍. It would be nice to also have a test included for this special logic so it doesn't break in the future.
Reviewed with ❤️ by PullRequest
Please make sure to fix the code formatting with prettier. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PullRequest Breakdown
Reviewable lines of change
+ 20
- 10
73% JavaScript
27% TSX
Type of change
Minor Update - These changes appear to be a minor update to existing functionality and features.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4956 +/- ##
==========================================
- Coverage 96.71% 96.34% -0.38%
==========================================
Files 28 28
Lines 3287 3307 +20
Branches 1368 1389 +21
==========================================
+ Hits 3179 3186 +7
- Misses 104 121 +17
+ Partials 4 0 -4 ☔ View full report in Codecov by Sentry. |
const handleInputClick = () => { | ||
inputRef.current.focus(); | ||
}; | ||
const inputRef = React.createRef(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
React is not defined. Curious if we can do this without importing React. None of the other examples needed this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to inactivity, PullRequest has cancelled this review job. You can reactivate the code review job from the PullRequest dashboard.
name: “Fix time input focus issue”
about: The time input in the "Custom time input" example does not focus when clicked on for the first time. It requires two clicks to focus.
title: "Custom time input needs 2 clicks to focus"
labels: ""
assignees: "@martijnrusschen"
Description
Linked issue: #4949
Problem
The time input in the “Custom time input” example does not focus when clicked on for the first time. It requires two clicks to focus.
Changes
I have fixed the issue by ensuring that the time input is focused on the first click.
Screenshots
In the right side when we click the time input bar, I have ensured that the time input is focused on the first click.
To reviewers
Contribution checklist