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

Calendar Mask: Selecting a value from the dropdown is not triggered on first click #6795

Closed
blubito-kristiyan-filevski-01 opened this issue Jun 26, 2024 · 6 comments · Fixed by #6972 or leoo1992/GeradorQRCode#85 · May be fixed by mtech-31-quemistry/quemistry_client_web#20
Assignees
Labels
Type: Bug Issue contains a defect related to a specific component.
Milestone

Comments

@blubito-kristiyan-filevski-01

Describe the bug

When you use a mask for the calendar and then click on a desired date in the same month, the first click does nothing, the second will accept the value change. Added a short video, but not sure if it showcases it clearly

Recording.2024-06-26.161821.mp4

NOTE: Changing the month or the year, will make the click work as intended. Only when the month hasn't changed, the first click is disregarded

Could potentially be related to issue #6792

Reproducer

https://stackblitz.com/edit/2baebw?file=src%2FApp.jsx

PrimeReact version

10.6.6

React version

17.x

Language

TypeScript

Build / Runtime

Create React App (CRA)

Browser(s)

Chrome 90

Steps to reproduce the behavior

  1. Use Calendar Mask
  2. Select by click a value without changing month/year
  3. Nothing happens
  4. Select again
  5. The value has been selected

  1. Use Calendar Mask
  2. Input manually a date
  3. Select a date from the dropdown without changing month/year
  4. The first click is disregarded
  5. Select again
  6. The value has been selected

Expected behavior

No response

@blubito-kristiyan-filevski-01 blubito-kristiyan-filevski-01 added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Jun 26, 2024
@frle10
Copy link
Contributor

frle10 commented Jul 29, 2024

This issue is the same one as #6042. My team is blocked by this in production because we need input mask and this keeps happening. I tried to fix it myself, but I've had some problems. I can provide my thoughts on a potential fix tomorrow to give this issue some momentum.

For now, I know that in version 10.3.2 this issue doesn't exist, and from 10.3.3 onwards it does. Something in the commits that went into 10.3.3 started causing this behavior, but I've had a problem pinpointing exactly what happened. I don't think it's a one line mistake but rather that it has to do with component architecture.

There are some event listeners being set when mask is on that might be overwriting some onClick behavior on the calendar compared to when the mask is not active. I can provide more details from my research in the next 2 days.

@melloware melloware added Type: Bug Issue contains a defect related to a specific component. and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Jul 29, 2024
@frle10
Copy link
Contributor

frle10 commented Aug 1, 2024

What I've tried doing is changing the following useUpdateEffect around line 3050 in Calendar.js file:

useUpdateEffect(() => {
    focusToFirstCell();
}, [currentView]);

And make it include another dependency, visible like so:

useUpdateEffect(() => {
    if (visible) {
        focusToFirstCell();
    }
}, [visible, currentView]);

This fixes the double click issue, but then what happens is when a user clicks on the calendar input, the focus is on the first date cell and then the user cannot start typing the date manually using the mask. So this in itself is not a solution, but maybe a starting point for thinking about it.

@frle10
Copy link
Contributor

frle10 commented Aug 1, 2024

When mask is present, the only additional code that runs is the following:

if (props.mask) {
    unbindMaskEvents = mask(inputRef.current, {
        mask: props.mask,
        slotChar: props.maskSlotChar,
        readOnly: props.readOnlyInput || props.disabled,
        onChange: (e) => {
            updateValueOnInput(e.originalEvent, e.value, () => {
                return false;
            });
        },
        onBlur: (e) => {
            updateValueOnInput(e, e.target.value);
        }
    }).unbindEvents;
}

The mask function comes from components/lib/utils/Mask.js file. There you can see the mask logic and also some event listeners being bound to inputRef.current:

const bindEvents = () => {
      el.addEventListener('focus', onFocus);
      el.addEventListener('blur', onBlur);
      el.addEventListener('keydown', onKeyDown);
      el.addEventListener('keypress', onKeyPress);
      el.addEventListener('input', onInput);
      el.addEventListener('paste', handleInputChange);
  };

There has to be something here that causes the problem, but I can't figure out so far what it is.

@melloware
Copy link
Member

@frle10 the focus on the first date cell is by ARIA/WCAG design. You cannot type while the dropdown is open because ARIA says if a popup is open you are interacting with the popup NOT with the input. So that is the correct behavior. If you don't want that behavior i recommend showOnFocus={false} see: https://primereact.org/calendar/#api.Calendar.props.showOnFocus

@frle10
Copy link
Contributor

frle10 commented Aug 2, 2024

@frle10 the focus on the first date cell is by ARIA/WCAG design. You cannot type while the dropdown is open because ARIA says if a popup is open you are interacting with the popup NOT with the input. So that is the correct behavior. If you don't want that behavior i recommend showOnFocus={false} see: https://primereact.org/calendar/#api.Calendar.props.showOnFocus

Thank you for this info, I didn't know this. That means that my attempt at a fix is actually correct? Because right now when the overlay opens you can still type in the input + you have to click twice on a date. With my fix focus would go to the overlay so you can't type + double click issue is fixed.

I'll make a PR for it if this is the desired behavior.

@melloware
Copy link
Member

Yes please @frle10 !

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