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

Consistency in naming: Width / Width Scale / Height Scale #6172

Closed
macandcheese opened this issue Dec 26, 2022 · 8 comments
Closed

Consistency in naming: Width / Width Scale / Height Scale #6172

macandcheese opened this issue Dec 26, 2022 · 8 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. Calcite (design) Issues logged by Calcite designers. enhancement Issues tied to a new feature or request. estimate - 3 A day or two of work, likely requires updates to tests. future breaking change Issues and pull requests with deprecation(s), requires a breaking change in a future milestone. p - low Issue is non core or affecting less that 10% of people using the library spike complete Issues that have a research spike completed and dev work can proceed

Comments

@macandcheese
Copy link
Contributor

Description

There are a few instances where we use "heightScale", "widthScale" to determine sizes for components, usually accepting s/m/l. There are also instances of "width" used frequently.

We should look at consolidating these patterns / naming...

heightScale - calcite-flow-item, calcite-panel, calcite-shell-center-row
detachedHeightScale - calcite-shell-panel
widthScale - -calcite-shell-panel, calcite-flow-item, calcite-panel

Many other components use width - some are s/m/l choice, others use full / auto / half etc.

Acceptance Criteria

A consistent naming and usage pattern is defined and implemented. I do think we should continue with the proposed changes to panel to remove height / width settings in general and have it behave as a block level element that fills the container it is placed in (most of the time, this would be shell panel, and its defined width / height scale or explicit value)

Relevant Info

No response

Which Component

Above

Example Use Case

No response

Esri team

Calcite (design)

@macandcheese macandcheese added enhancement Issues tied to a new feature or request. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Dec 26, 2022
@github-actions github-actions bot added the Calcite (design) Issues logged by Calcite designers. label Dec 26, 2022
@ashetland
Copy link

Agree, this would be a good one to dig into.

@geospatialem geospatialem added p - low Issue is non core or affecting less that 10% of people using the library estimate - 3 A day or two of work, likely requires updates to tests. needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. and removed needs triage Planning workflow - pending design/dev review. labels Jan 22, 2024
@geospatialem
Copy link
Member

For 2024 we should work towards prop deprecation, and for the 2025 breaking change consider removal of any of the deprecated components.

@brittneytewks brittneytewks removed the needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. label Apr 3, 2024
@Elijbet Elijbet self-assigned this Aug 26, 2024
@Elijbet Elijbet added 1 - assigned Issues that are assigned to a sprint and a team member. spike Issues that need quick investigations for time estimations, prioritization, or a quick assessment. and removed 0 - new New issues that need assignment. labels Aug 26, 2024
@Elijbet
Copy link
Contributor

Elijbet commented Aug 27, 2024

I scanned through components and made a list of what each uses. The majority (50+) of components only use scale. Some of the components use width/widthScale in addition:

Dialog
Dropdown
Modal

/** Specifies the size of the component. */	
@Prop({ reflect: true }) scale: Scale = "m";

/** Specifies the width of the component. */
@Prop({ reflect: true }) widthScale: Scale = "m";

Notice
Segmented-control
Select
Split-button


/** Specifies the size of the component. */	
@Prop({ reflect: true }) scale: Scale = "m";

/** Specifies the width of the component. */
@Prop({ reflect: true }) width: Width = "auto";

Button
Tile-select

@Prop({ reflect: true }) width: Width = "auto";

Shell-Panel

@Prop({ reflect: true }) widthScale: Scale = "m";

Sheet

/**
* When `position` is `"inline-start"` or `"inline-end"`, specifies the width of the component.
*/
@Prop({ reflect: true }) widthScale: Scale = "m";
/**
* When `position` is `"block-start"` or `"block-end"`, specifies the height of the component.
*/
@Prop({ reflect: true }) heightScale: Scale = "m";

Shell-center-row

/**
* Specifies the maximum height of the component.
*/
  @Prop({ reflect: true }) heightScale: Scale = "s";

Shell-pane

/**
* When `displayMode` is `float-content` or `float`, specifies the maximum height of the component.
*
* @deprecated Use `heightScale` instead.
*/
@Prop({ reflect: true }) detachedHeightScale: Scale;
/**
* When `layout` is `horizontal`, specifies the maximum height of the component.
*/
@Prop({ reflect: true }) heightScale: Scale;
/**
* When `layout` is `vertical`, specifies the width of the component.
*/
@Prop({ reflect: true }) widthScale: Scale = "m";

@Elijbet
Copy link
Contributor

Elijbet commented Aug 27, 2024

Related issues to consider:
Epic: Audit scale across components#7082
Includes adding scales to components:
Block
Block-section
Panel
Flow-item
List
Menu
Menu-item

[Design] Consistent width-scale , height-scale values across components #7616

@Elijbet Elijbet added 2 - in development Issues that are actively being worked on. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels Aug 27, 2024
@Elijbet
Copy link
Contributor

Elijbet commented Aug 30, 2024

Currently:

  • widthScale has been used when the width has a set of defaults, that use s/m/l as values
  • width has been used when the width has a set of defaults, but are not s/m/l - instead are auto / half / full.



The discussion width vs widthScale is leaning towards single prop width with s / m / l / auto / full  as options, since they are used in a mutually exclusive manner. The values width accepts would still depend on the component.

There is no great use case for using half value outside of two adjacent elements each taking up 50% of the container, in which case we’ll just set them to full.

@Elijbet Elijbet added spike complete Issues that have a research spike completed and dev work can proceed and removed spike Issues that need quick investigations for time estimations, prioritization, or a quick assessment. labels Aug 30, 2024
@github-actions github-actions bot added 0 - new New issues that need assignment. and removed 2 - in development Issues that are actively being worked on. labels Aug 30, 2024
@github-actions github-actions bot removed this from the 2024-09-24 - Sep Main release milestone Aug 30, 2024
Copy link
Contributor

cc @geospatialem, @brittneytewks

@Elijbet Elijbet self-assigned this Aug 30, 2024
@Elijbet Elijbet added 2 - in development Issues that are actively being worked on. 0 - new New issues that need assignment. and removed 0 - new New issues that need assignment. 2 - in development Issues that are actively being worked on. labels Aug 30, 2024
@Elijbet Elijbet added this to the 2024-09-24 - Sep Main release milestone Aug 30, 2024
@geospatialem geospatialem added 2 - in development Issues that are actively being worked on. and removed 0 - new New issues that need assignment. needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. labels Aug 31, 2024
Elijbet added a commit that referenced this issue Nov 25, 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 added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels Nov 25, 2024
@github-actions github-actions bot assigned geospatialem and DitwanP and unassigned Elijbet Nov 25, 2024
Copy link
Contributor

Installed and assigned for verification.

@DitwanP
Copy link
Contributor

DitwanP commented Nov 25, 2024

🍡 Verified locally

@DitwanP DitwanP closed this as completed Nov 25, 2024
@geospatialem geospatialem added future breaking change Issues and pull requests with deprecation(s), requires a breaking change in a future milestone. 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. Calcite (design) Issues logged by Calcite designers. enhancement Issues tied to a new feature or request. estimate - 3 A day or two of work, likely requires updates to tests. future breaking change Issues and pull requests with deprecation(s), requires a breaking change in a future milestone. p - low Issue is non core or affecting less that 10% of people using the library spike complete Issues that have a research spike completed and dev work can proceed
Projects
None yet
Development

No branches or pull requests

7 participants