Skip to content

Conversation

liamdebeasi
Copy link
Contributor

@liamdebeasi liamdebeasi commented Dec 1, 2023

Issue number: Internal


What is the current behavior?

We'd like to introduce a new API to the picker column to allow developers to manipulate the picker options right in the DOM. Currently developers need to pass an object which is quite cumbersome as you need to re-pass that object anytime you want to change a single option.

What is the new behavior?

This PR does some of the prep work to make integrating the picker column option into picker column easier. It does the following things:

  1. Copies the base button styles from picker-column over to picker-column-option
  2. Creates active and disabled states (plus styles)
  3. Add tests for base styles

Note: There are duplicate styles across picker-column and picker-column-option that will be cleaned up once picker-column-option is integrated and shown to be functional.

Does this introduce a breaking change?

  • Yes
  • No

Other information

@github-actions github-actions bot added the package: core @ionic/core package label Dec 1, 2023
* Default options are: `"primary"`, `"secondary"`, `"tertiary"`, `"success"`, `"warning"`, `"danger"`, `"light"`, `"medium"`, and `"dark"`.
* For more information on colors, see [theming](/docs/theming/basics).
*/
@Prop({ reflect: true }) color?: Color = 'primary';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is currently on the picker-column component but will be removed once picker-column-option is fully integrated.

@github-actions github-actions bot added package: angular @ionic/angular package package: vue @ionic/vue package labels Dec 1, 2023
@liamdebeasi liamdebeasi marked this pull request as ready for review December 1, 2023 18:00
Copy link
Contributor

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -0,0 +1 @@
@import "./picker-column-option.scss";
Copy link
Contributor

Choose a reason for hiding this comment

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

Our existing components leave off the file extension with importing another SASS file.

@@ -0,0 +1,5 @@
@import "./picker-column-option.scss";
Copy link
Contributor

Choose a reason for hiding this comment

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

Our existing components leave off the file extension with importing another SASS file.

Copy link
Contributor

@sean-perkins sean-perkins left a comment

Choose a reason for hiding this comment

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

Few minor clean-up suggestions, rest looks good.

@liamdebeasi liamdebeasi merged commit 91d0e26 into FW-5580 Dec 5, 2023
@liamdebeasi liamdebeasi deleted the ld/5580-base branch December 5, 2023 19:40
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 package: vue @ionic/vue package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants