Skip to content

Comments

feat(picker-column): add 'showName' property#23576

Closed
Giulio-91 wants to merge 1 commit intoionic-team:mainfrom
Giulio-91:ionic-picker
Closed

feat(picker-column): add 'showName' property#23576
Giulio-91 wants to merge 1 commit intoionic-team:mainfrom
Giulio-91:ionic-picker

Conversation

@Giulio-91
Copy link

@Giulio-91 Giulio-91 commented Jul 4, 2021

add the possibility tho display the column name by setting the showName column property to true

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (npm run build) was run locally and any changes were pushed
  • Lint (npm run lint) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Issue Number: N/A

What is the new behavior?

  • now it is possible to show the column name by setting the showName column property to true

Does this introduce a breaking change?

  • Yes
  • No

Other information

add the possibility tho display the column name by setting the showName column property to true
@github-actions github-actions bot added the package: core @ionic/core package label Jul 4, 2021
@liamdebeasi
Copy link
Contributor

Thanks for the PR! Is there an associated issue for this PR?

@Giulio-91
Copy link
Author

Thanks for the PR! Is there an associated issue for this PR?

No. Should I create it?

@liamdebeasi
Copy link
Contributor

If there isn't one already that's fine. Could you please explain the use case for this feature?

@Giulio-91
Copy link
Author

If there isn't one already that's fine. Could you please explain the use case for this feature?

Yes of course. I needed to show the column name of the picker.

So I added the "showName" property in the 'PickerColumn' interface. If it is set to true, the column name is shown on the top of the column.

I also added an example in the test script.

@liamdebeasi
Copy link
Contributor

Thanks for the clarification and apologies for the delay. What is the benefit of showing the column name instead of using prefix or suffix? In practice, the prefix and suffix properties are what describe the columns. Here's an example from the stock iOS clock app:

IMG_8516

I think the addition of this API is a bit confusing since it lets you describe the column in two different ways. Here is a screenshot of the example you added:

image

The "hours" suffix describes the column, so having showName here is redundant in my opinion.

@liamdebeasi
Copy link
Contributor

Thanks for the PR! I discussed this with the team and we are unable to accept your PR at this time. We feel that the existing suffix and prefix properties already provide the functionality that the proposed showName property would provide.

As I mentioned in #23576 (comment), native iOS apps use the suffix and prefix features to assign labels to picker columns. Adding the showName property to Ionic Framework would allow developers describe a column in two different ways which we feel may lead to a confusing user experience.

In the future we recommend opening an issue first before creating a PR as it lets us discuss the scope of a feature before any programming work begins. Please let me know if you have any questions. Thank you!

@liamdebeasi liamdebeasi closed this Aug 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: core @ionic/core package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants