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

Focus on datepicker footer triggers extra selects #3444

Closed
Nosfistis opened this issue Oct 31, 2019 · 5 comments · Fixed by #3461
Closed

Focus on datepicker footer triggers extra selects #3444

Nosfistis opened this issue Oct 31, 2019 · 5 comments · Fixed by #3461

Comments

@Nosfistis
Copy link

Bug description:

When using a datepicker with a custom footer, if that footer contains focusable elements (e.g. inputs), navigating between those elements with keyboard tab causes select events which are invalid. The selection payload is not an NgbDate instance, but a JS Event with type: 'select'.

Link to minimally-working StackBlitz that reproduces the issue:

https://stackblitz.com/edit/ng-bootstrap-footer-focus

Versions of Angular, ng-bootstrap and Bootstrap:

Angular: 8.2.12

ng-bootstrap: 5.1.2

Bootstrap: 4.3.0

@fbasso
Copy link
Member

fbasso commented Oct 31, 2019

Thanks for opening this your issue @Nosfistis .

After a quick investigation, it seems it's because 'select' is a native event which is triggered by the browser when the input is focused, which is the same name of the one used to select a day. It is not supported by all the browsers.

https://developer.mozilla.org/en-US/docs/Web/API/GlobalEventHandlers/onselect

@Nosfistis
Copy link
Author

I did not think of a native event at all!

It confused my why it's not named (dateSelect), same as the directive one. As a workaround. I am using a filter on the event, checking if it returns payload of type NgbDate.

@maxokorokov
Copy link
Member

maxokorokov commented Nov 4, 2019

Hey, @Nosfistis. Thanks for reporting this.

  • Datepicker was created before the InputDatepicker
  • Datepicker could never raise select event, so the output was named (select)
  • InputDatepicker was added on the <input> element, so we had to name similar output (dateSelect)
  • then we added a footer to the Datepicker and got this issue :)

Anyway I suggest we:

  1. introduce (dateSelect) to the datepicker
  2. deprecate (select)
  3. remove (select) in the next major release

@maxokorokov maxokorokov self-assigned this Nov 7, 2019
@maxokorokov maxokorokov added this to the 5.2.0 milestone Nov 7, 2019
peterblazejewicz added a commit to peterblazejewicz/ng-bootstrap that referenced this issue Nov 9, 2019
This commit changes name of the component custom event to avoid
conflicts with native 'select' event.
Existing event name is marked as deprecated for backward compatibility.

Fixes ng-bootstrap#3444
peterblazejewicz added a commit to peterblazejewicz/ng-bootstrap that referenced this issue Nov 9, 2019
This commit changes name of the component custom event to avoid
conflicts with native 'select' event.
Existing event name is marked as deprecated for backward compatibility.

Fixes ng-bootstrap#3444
peterblazejewicz added a commit to peterblazejewicz/ng-bootstrap that referenced this issue Nov 11, 2019
This commit changes name of the component custom event to avoid
conflicts with native 'select' event.
Existing event name is marked as deprecated for backward compatibility.

Fixes ng-bootstrap#3444
maxokorokov pushed a commit to peterblazejewicz/ng-bootstrap that referenced this issue Jan 9, 2020
This commit changes name of the component custom event to avoid
conflicts with native 'select' event.

Existing event name will be marked as deprecated in v6.

Fixes ng-bootstrap#3444
maxokorokov pushed a commit that referenced this issue Jan 9, 2020
This commit changes name of the component custom event to avoid
conflicts with native 'select' event.

Existing event name will be marked as deprecated in v6.

Fixes #3444
@maxokorokov
Copy link
Member

maxokorokov commented Jan 9, 2020

Not yet fully fixed actually, will keep it open for 6.0.
Since 5.2.0 there is a new (dateSelect) output to avoid collisions

@maxokorokov maxokorokov reopened this Jan 9, 2020
@maxokorokov maxokorokov modified the milestones: 5.2.0, 6.0 Jan 9, 2020
@maxokorokov
Copy link
Member

Closed in #3611

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

Successfully merging a pull request may close this issue.

3 participants