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.

Sign up for GitHub

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

Jump to bottom

Last item not fully visible when scrolling in time when using portalId in scrollable div #4290

Closed
1Jesper1 opened this issue Oct 5, 2023 · 6 comments · Fixed by qburst/react-datepicker-3#3, #4296 or jones58/personal-knowledge-wiki#29 · May be fixed by rowidanagah/EduMentorMate#9
Closed

Last item not fully visible when scrolling in time when using portalId in scrollable div #4290

1Jesper1 opened this issue Oct 5, 2023 · 6 comments · Fixed by qburst/react-datepicker-3#3, #4296 or jones58/personal-knowledge-wiki#29 · May be fixed by rowidanagah/EduMentorMate#9

Comments

@1Jesper1
Copy link

1Jesper1 commented Oct 5, 2023

Describe the bug
Last item not fully visible when scrolling in time when using portalId in scrollable div

To Reproduce

() => {
  const [startDate, setStartDate] = useState(new Date('02-02=2023 06:00'));
  return (
  <div>
  <div id='test'></div>
    <DatePicker
      portalId={'test'}
      selected={startDate}
      onChange={(date) => setStartDate(date)}
      showTimeSelect
    />
     </div>
  );
};

Expected behavior
Should show the full height of last time item.

Screenshots
image

Desktop (please complete the following information):

  • OS: Windows
  • BrowserChrome
  • Version react-datepicker 4.10.0
@balajis-qb
Copy link

balajis-qb commented Oct 6, 2023 •
edited
Loading

Hi @1Jesper1 ,
I couldn't able to reproduce the error. I can able to see the time list fully

time list scroll issue

Is there any case in which the last item is getting hidden? Because I tried with the code you shared and I can't able to reproduce it

@1Jesper1
Copy link
Author

1Jesper1 commented Oct 6, 2023

@balajis-qb
Please try:

() => {
  const [startDate, setStartDate] = useState(new Date('02-02=2023 11:30 pm'));
  return (
  <div>
  <div id='test'></div>
    <DatePicker
      portalId={'test'}
      selected={startDate}
      onChange={(date) => setStartDate(date)}
      showTimeSelect
    />
     </div>
  );
};

@1Jesper1
Copy link
Author

1Jesper1 commented Oct 6, 2023

image
Also have the bug in Firefox.

@balajis-qb
Copy link

balajis-qb commented Oct 6, 2023 •
edited
Loading

Yeah, I got the issue. Initially I misunderstood the issue. You are mentioning about the initial view right? when we open the date picker with the last time item as selected, in that case we have to manually scroll to the selected time. Yes I got the issue, I can also reproduce it. Will work on it

balajis-qb pushed a commit to qburst/react-datepicker-3 that referenced this issue Oct 6, 2023
fix: Ensure accurate scroll position on time component mounts
549e6ce 
This commit addresses an issue where the scroll position was not accurately set when the component mounted.  To resolve this, we have incorporated the use of requestAnimationFrame to delay the scroll update until after the component has fully rendered and layout calculations are complete.  This ensures consistent and precise scroll behavior.

Closes Hacker0x01#4290
balajis-qb pushed a commit to qburst/react-datepicker-3 that referenced this issue Oct 7, 2023
test: Update scroll position test cases to handle requestAnimationFrame
88e3641 
In the time component, the scroll position is set using requestAnimationFrame. To ensure the accurate testing, this commit updates the test case to use waitFor from React Testing Library, allowing the test to wait for the scroll update to complete before making assertions about the scroll position.  The test now provides more reliable results and ensures that the scroll behavior is properly tested.

Closes Hacker0x01#4290
balajis-qb pushed a commit to qburst/react-datepicker-3 that referenced this issue Oct 7, 2023
refactor: Ensure Time component ref availability for requestAnimation…
efeeccc 
…Frame

In the Time component, this commit refactors the code to ensure that the component ref is available when used inside a requestAnimationFrame callback.  By checking the ref availability, the issue of rendering failures is resolved, particularly during testing with asynchronous behavior.

Closes Hacker0x01#4290
This was referenced Oct 7, 2023
Merged
Merged
@balajis-qb
Copy link

balajis-qb commented Oct 7, 2023 •
edited
Loading

Hi,
Actually the mentioned scroll issue occurs all the times regardless of the usage of the portalId.

I fixed the issue in my PR #4296 . This PR also fixes the other scroll related issue #4289. Kindly review my fix and let me know your review comments.

@balajis-qb balajis-qb mentioned this issue Oct 7, 2023
Closed
@1Jesper1
Copy link
Author

1Jesper1 commented Oct 8, 2023

Hi, Actually the mentioned scroll issue occurs all the times regardless of the usage of the portalId.

I fixed the issue in my PR #4296 . This PR also fixes the other scroll related issue #4289. Kindly review my fix and let me know your review comments.

Thanks for the fast fix for both issues. PR is detailed!

martijnrusschen added a commit that referenced this issue Oct 8, 2023
@martijnrusschen
Merge pull request #4296 from qburst/issue-4290/fix/last-time-list-it…
64d750a 
…em-scroll-issue

Fix #4290 and #4289: Time component scroll issue
@snyk-io snyk-io bot mentioned this issue Jun 11, 2024
Closed
@jones58 jones58 mentioned this issue Aug 6, 2024
Merged
@rowidanagah rowidanagah mentioned this issue Aug 25, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@1Jesper1 @balajis-qb