Skip to content

feat: deprecate widthScale/heightScale in favor of width/height #10786

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

Conversation

Elijbet
Copy link
Contributor

@Elijbet Elijbet commented Nov 19, 2024

Related Issue: #6172

Summary

Refactor to consolidate width/height and widthScale/heightScale into a single property width with s / m / l / auto / full as options, and height with s / m / l. Deprecate widthScale and heightScale properties and the half value.

Components addressed:

  • button
  • dialog
  • dropdown
  • notice
  • segmented-control
  • select
  • sheet
  • shell-panel
  • split-button

deprecate: deprecate widthScale/heightScale properties in favor of width/height.

@Elijbet Elijbet marked this pull request as draft November 19, 2024 00:09
@Elijbet Elijbet force-pushed the elijbet/6172-deprecate-widthScale/heightScale-in-favor-of-width/height branch from fbddf02 to df2138b Compare November 19, 2024 01:13
@Elijbet Elijbet added pr ready for visual snapshots Adding this label will run visual snapshot testing. refactor Issues tied to code that needs to be significantly reworked. labels Nov 19, 2024
@Elijbet Elijbet added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Nov 20, 2024
@Elijbet Elijbet added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Nov 22, 2024
@Elijbet Elijbet added pr ready for visual snapshots Adding this label will run visual snapshot testing. refactor Issues tied to code that needs to be significantly reworked. and removed refactor Issues tied to code that needs to be significantly reworked. pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Nov 22, 2024
@Elijbet Elijbet marked this pull request as ready for review November 22, 2024 20:40
Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

Awesome stuff, @Elijbet! 🎉

Before merging, could you update the PR to only list components with width/height-prop-related updates?

The commit type might also need to be feat since we’re adding new properties for consistency. If there weren’t new props involved, deprecate would’ve been perfect. @geospatialem @DitwanP

Once the comments are addressed, this should be good to go!

@jcfranco
Copy link
Member

The commit type might also need to be feat since we’re adding new properties for consistency. If there weren’t new props involved, deprecate would’ve been perfect. @geospatialem @DitwanP

Actually, we could leave this as is and use commit overrides to follow the steps outlined here.

Sidebar: @DitwanP is the above recommendation in our internal doc? I wasn't able find it.

@Elijbet Elijbet changed the title deprecate: deprecate widthScale/heightScale in favor of width/height feat: deprecate widthScale/heightScale in favor of width/height Nov 25, 2024
@github-actions github-actions bot added the enhancement Issues tied to a new feature or request. label Nov 25, 2024
@DitwanP
Copy link
Contributor

DitwanP commented Nov 25, 2024

The commit type might also need to be feat since we’re adding new properties for consistency. If there weren’t new props involved, deprecate would’ve been perfect. @geospatialem @DitwanP

Actually, we could leave this as is and use commit overrides to follow the steps outlined here.

Sidebar: @DitwanP is the above recommendation in our internal doc? I wasn't able find it.

We could indeed have the commit type as feat and then add a deprecation commit message to the body in order to list that separately on the changelog if that's what you're asking. And here's the link to the wiki section that covers this.

@Elijbet Elijbet added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Nov 25, 2024
@Elijbet Elijbet merged commit deece6d into dev Nov 25, 2024
13 checks passed
@Elijbet Elijbet deleted the elijbet/6172-deprecate-widthScale/heightScale-in-favor-of-width/height branch November 25, 2024 19:43
@jcfranco
Copy link
Member

@DitwanP Thanks for the link!

benelan pushed a commit that referenced this pull request Feb 8, 2025
**Related Issue:** #6172

## Summary
Refactor to consolidate `width/height` and `widthScale/heightScale` into
a single property `width` with `s / m / l / auto / full` as options, and
`height` with `s / m / l`. Deprecate `widthScale` and `heightScale`
properties and the `half` value.

Components addressed:

- `button`
- `dialog`
- `dropdown`
- `notice`
- `segmented-control`
- `select`
- `sheet`
- `shell-panel`
- `split-button`

deprecate: deprecate widthScale/heightScale properties in favor of
width/height.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues tied to a new feature or request. pr ready for visual snapshots Adding this label will run visual snapshot testing. refactor Issues tied to code that needs to be significantly reworked.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants