Skip to content

Conversation

liamdebeasi
Copy link
Contributor

@liamdebeasi liamdebeasi commented Dec 6, 2023

Issue number: N/A


What is the current behavior?

As noted in #28622, keyboard entry was not migrated to minimize the amount of code changes that needed to be reviewed at once. Keyboard entry would be updated in a separate PR.

What is the new behavior?

  • This PR updates the keyboard entry implementation so it searches ion-picker-column-option elements in a picker column instead of the item property on the column.
  • Keyboard entry tests that were previously skipped have been re-enabled.

Does this introduce a breaking change?

  • Yes
  • No

Other information

@github-actions github-actions bot added the package: core @ionic/core package label Dec 6, 2023
const findItemFromCompleteValue = values.find(({ text }) => {
const parsedText = text.replace(/^0+(?=[1-9])|0+(?=0$)/, '');
const findItemFromCompleteValue = options.find(({ textContent }) => {
const parsedText = textContent!.replace(/^0+(?=[1-9])|0+(?=0$)/, '');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keyboard entry is currently only used inside of Datetime where we guarantee textContent is set. If we end up exposing this feature publicly we should revisit this assumption.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we move this to a comment directly in the code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in a00f563

@github-actions github-actions bot added the package: angular @ionic/angular package label Dec 6, 2023
@liamdebeasi liamdebeasi marked this pull request as ready for review December 6, 2023 17:28
@liamdebeasi liamdebeasi requested review from a team, sean-perkins and thetaPC as code owners December 6, 2023 17:28
const findItemFromCompleteValue = values.find(({ text }) => {
const parsedText = text.replace(/^0+(?=[1-9])|0+(?=0$)/, '');
const findItemFromCompleteValue = options.find(({ textContent }) => {
const parsedText = textContent!.replace(/^0+(?=[1-9])|0+(?=0$)/, '');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we move this to a comment directly in the code?

Base automatically changed from ld/5580-integration to FW-5580 December 6, 2023 21:30
@liamdebeasi liamdebeasi merged commit 4493a64 into FW-5580 Dec 6, 2023
@liamdebeasi liamdebeasi deleted the ld/5580-keyboard-entry branch December 6, 2023 21:33
@mlakmal
Copy link

mlakmal commented Mar 22, 2024

@liamdebeasi any idea when this update will be released?

im not seeing keyboard navigation working for standalone ion-picker in v7.8.1

@thetaPC
Copy link
Contributor

thetaPC commented Mar 22, 2024

@mlakmal This fix can be found in v8 beta! If you want to test it out, then it's recommended to read the upgrade guide to v8 beta on the Frameworks docs site.

@lakmal-gentem
Copy link

@mlakmal This fix can be found in v8 beta! If you want to test it out, then it's recommended to read the upgrade guide to v8 beta on the Frameworks docs site.

Thanks for the info. But we have a hard dependency with angular 15. Are thr any plans to fix this in v7 ?

@liamdebeasi
Copy link
Contributor Author

We don't plan to backport this feature to ion-picker in Ionic v7. However, this picker (and the keyboard functionality) is available as part of ion-datetime in v7.

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

Labels

package: angular @ionic/angular package package: core @ionic/core package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants