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

[BUG] - Date Picker Showing Wrong Year #3785

Open
buchananwill opened this issue Sep 19, 2024 · 13 comments · May be fixed by #3789
Open

[BUG] - Date Picker Showing Wrong Year #3785

buchananwill opened this issue Sep 19, 2024 · 13 comments · May be fixed by #3789
Assignees
@buchananwill
Labels
🐛 Type: Bug Something isn't working

Comments

@buchananwill
Copy link

NextUI Version

2.4.6

Describe the bug

image

Speaks for itself really... 1987 is in the scroller, but 1986 in the focus. Which is it going to pick? Not clear...

A few scrolls later and this even more confusing collection of conflicts appears:

image

This was my code (using react-hook-form):

  const dateOfBirth = watch('dateOfBirth');

  const setDateValue = useCallback(
    (value: CalendarDate) => {
      const isoString = value
        .toDate(getLocalTimeZone())
        .toISOString()
        .split('T')[0];
      setValue('dateOfBirth', isoString);
    },
    [setValue]
  );

          <DatePicker
            name={'dateOfBirth'}
            aria-label={'Date of Birth'}
            label={'Date of Birth'}
            value={parseDate(dateOfBirth)}
            showMonthAndYearPickers={true}
            onChange={setDateValue}
          />

Your Example Website or App

No response

Steps to Reproduce the Bug or Issue

Use the date picker in a component. Could it be related to using a controlled value?

Expected behavior

I would expect the highlighted, focused and selected values to all be in agreement.

Screenshots or Videos

No response

Operating System Version

  • Windows 10 Home V.22H2

Browser

Chrome
@linear Linear
Copy link

linear bot commented Sep 19, 2024

ENG-1377 [BUG] - Date Picker Showing Wrong Year

@buchananwill
Copy link
Author

buchananwill commented Sep 19, 2024 •
edited
Loading

I was also getting this error on the console at the time:

Blocked aria-hidden on a <div> element because the element that just received focus must not be hidden from assistive technology users. Avoid using aria-hidden on a focused element or its ancestor. Consider using the inert attribute instead, which will also prevent focus. For more details, see the aria-hidden section of the WAI-ARIA specification at https://w3c.github.io/aria/#aria-hidden. <div class=​"flex flex-col relative overflow-hidden h-auto text-foreground box-border bg-content1 outline-none data-[focus-visible=true]​:​z-10 data-[focus-visible=true]​:​outline-2 data-[focus-visible=true]​:​outline-focus data-[focus-visible=true]​:​outline-offset-2 shadow-medium rounded-large transition-transform-background motion-reduce:​transition-none mt-8 w-64" tabindex=​"-1">​flex</div>​

@ryo-manba ryo-manba added the 🐛 Type: Bug Something isn't working label Sep 21, 2024
@subhamengine
Copy link

Hey, i would like to work on this issue.

@wingkwong
Copy link
Member

@subhamengine go ahead

@subhamengine
Copy link

sure. How can I reproduce it?

@buchananwill

This comment was marked as resolved.

Sign in to view
@buchananwill
Copy link
Author

The only concrete thing I've determined, is that the set syncing between the month/year picker and main view is off-by-one, and glitches every time it is shown/hidden.

bad.calendar.mp4

@buchananwill
Copy link
Author

Here is a code sandbox minimum project though. The tailwind is via CDN, because Tailwind -/-> codesandbox, but the behaviour is the same:

https://codesandbox.io/p/sandbox/j864t8

@buchananwill
Copy link
Author

Two hours of breakpoint stepping later, and I believe I have the solution. Correct me if I've misunderstood any of this code.

This function is doing the actual data updating, by checking which div intersects the little grey highlight bar:

const handleListScroll = useCallback(
    (e: Event, highlightEl: HTMLElement | null, list: CalendarPickerListType) => {
      if (!(e.target instanceof HTMLElement)) return;

      const map = getItemsRefMap(list === "months" ? monthsItemsRef : yearsItemsRef);

      const items = Array.from(map.values());

It loops through the items in ascending order, and stops when it finds the intersecting div...

      const item = items.find((itemEl) => {
        const rect1 = itemEl.getBoundingClientRect();
        const rect2 = highlightEl?.getBoundingClientRect();

        if (!rect2) {
          return false;
        }

...Except when it doesn't. because the previous div is able to slightly overlap with the highlight as well. At 125% zoom on my PC, the bug goes away. Only at that zoom setting though. The rest of the time it's broken.

        return areRectsIntersecting(rect1, rect2);
      });

      const itemValue = Number(item?.getAttribute("data-value"));

      if (!itemValue) return;

Having stopped one element too early, the focusedDate is set to the wrong value.

      let date = state.focusedDate.set(list === "months" ? {month: itemValue} : {year: itemValue});

      state.setFocusedDate(date);
    },
    [state, isHeaderExpanded],
  );

My proposal:

Delete both these functions:

 const handleListScroll = useCallback(
    (e: Event, highlightEl: HTMLElement | null, list: CalendarPickerListType) => {
      if (!(e.target instanceof HTMLElement)) return;

      const map = getItemsRefMap(list === "months" ? monthsItemsRef : yearsItemsRef);

      const items = Array.from(map.values());

      const item = items.find((itemEl) => {
        const rect1 = itemEl.getBoundingClientRect();
        const rect2 = highlightEl?.getBoundingClientRect();

        if (!rect2) {
          return false;
        }

        return areRectsIntersecting(rect1, rect2);
      });

      const itemValue = Number(item?.getAttribute("data-value"));

      if (!itemValue) return;

      let date = state.focusedDate.set(list === "months" ? {month: itemValue} : {year: itemValue});

      state.setFocusedDate(date);
    },
    [state, isHeaderExpanded],
  );

  useEffect(() => {
    // add scroll event listener to monthsListRef
    const monthsList = monthsListRef.current;
    const yearsList = yearsListRef.current;
    const highlightEl = highlightRef.current;

    if (!highlightEl) return;

    const debouncedHandleMonthsScroll = debounce(
      (e: Event) => handleListScroll(e, highlightEl, "months"),
      SCROLL_DEBOUNCE_TIME,
    );
    const debouncedHandleYearsScroll = debounce(
      (e: Event) => handleListScroll(e, highlightEl, "years"),
      SCROLL_DEBOUNCE_TIME,
    );

    monthsList?.addEventListener("scroll", debouncedHandleMonthsScroll);
    yearsList?.addEventListener("scroll", debouncedHandleYearsScroll);

    return () => {
      if (debouncedHandleMonthsScroll) {
        monthsList?.removeEventListener("scroll", debouncedHandleMonthsScroll);
      }
      if (debouncedHandleYearsScroll) {
        yearsList?.removeEventListener("scroll", debouncedHandleYearsScroll);
      }
    };
  }, [handleListScroll]);

Update the data directly, when we already had the exact UI element that the user interacted with, which has the relevant data bound to it

  function scrollTo(value, list, smooth = true) {
    
    const mapListRef = list === "months" ? monthsItemsRef : yearsItemsRef;
    const listRef = list === "months" ? monthsListRef : yearsListRef;
// Update the focusedDate directly here.
    let date2 = state.focusedDate.set(list === "months" ? { month: itemValue } : { year: itemValue });
    state.setFocusedDate(date2);
    const map = getItemsRefMap(mapListRef);
    const node = map.get(value);
    if (!node)
      return;
    scrollIntoView(node, {
      scrollMode: "always",
      behavior: smooth ? "smooth" : "auto",
      boundary: listRef.current
    });
  }

buchananwill added a commit to buchananwill/nextui-wb-dev that referenced this issue Sep 22, 2024
@buchananwill
fix(use-calendar-picker): fixed the UI month/year sync (nextui-org#3785)
5f35a84
buchananwill added a commit to buchananwill/nextui-wb-dev that referenced this issue Sep 22, 2024
@buchananwill
fix(use-calendar-picker): fixed the UI month/year sync (nextui-org#3785)
66108cd
buchananwill added a commit to buchananwill/nextui-wb-dev that referenced this issue Sep 23, 2024
@buchananwill
fix(use-calendar-picker): added test for the fixed behaviour (nextui-…
d6b00bc 
…org#3785)
buchananwill added a commit to buchananwill/nextui-wb-dev that referenced this issue Sep 23, 2024
@buchananwill
fix(use-calendar-picker): changeset (nextui-org#3785)
e30ae3c
@buchananwill buchananwill linked a pull request Sep 23, 2024 that will close this issue
Open
@buchananwill
Copy link
Author

buchananwill commented Sep 25, 2024 via email

@luka-on-gh
Copy link

Hi. Any updates on this issue and when can we expect the fixes to be live?

@buchananwill
Copy link
Author

I authored a fix and finished working on it two weeks ago. As far as I was able to assess, all the behaviours are now working correctly in that PR. I haven't had any further feedback from @wingkwong or @ryo-manba since then. They may be able to answer when it might be included in a public release.

@ryo-manba
Copy link
Member

@buchananwill
Sorry for the delayed review. I've left some comment, so please take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@buchananwill buchananwill

Labels
🐛 Type: Bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(date-picker): month and year picker scroller buchananwill/nextui-wb-dev
5 participants
@wingkwong @ryo-manba @subhamengine @luka-on-gh @buchananwill