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

Action: deprecate "compact" property #3859

Closed
asangma opened this issue Jan 4, 2022 · 17 comments
Closed

Action: deprecate "compact" property #3859

asangma opened this issue Jan 4, 2022 · 17 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. enhancement Issues tied to a new feature or request. estimate - 2 Small fix or update, may require updates to tests. future breaking change Issues and pull requests with deprecation(s), requires a breaking change in a future milestone. low risk Issues with low risk for consideration in low risk milestones p - low Issue is non core or affecting less that 10% of people using the library

Comments

@asangma
Copy link
Contributor

asangma commented Jan 4, 2022

Description

Deprecate the compact property.

Acceptance Criteria

Include updates to screener and and e2e tests.

Relevant Info

Which Component

CalciteAction

Example Use Case

@asangma asangma 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 Jan 4, 2022
@asangma
Copy link
Contributor Author

asangma commented Jan 4, 2022

cc @AdelheidF @kevindoshier Do you know if this property is being used?

@AdelheidF
Copy link

Yes, it's used in filter and layer-override.

@AdelheidF
Copy link

AdelheidF commented Jan 4, 2022

used e.g. here:
image

without it:
image

@driskull
Copy link
Member

driskull commented Jan 4, 2022

This property might be ok to have as it just adjusts internal width.

@macandcheese
Copy link
Contributor

I think this should be internal if needed at all. In the examples above, using "compact" isn't aligning the x with the carets above or below it anyway.

@AdelheidF
Copy link

At least it shows at the end of its 'row'. If we take compact away the x just appears somewhere in the middle of the 'row'.

@geospatialem geospatialem added design Issues that need design consultation prior to development. 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 labels Aug 21, 2023
@geospatialem
Copy link
Member

Similar to tile's embed property. If the above proposal is adopted, we should revisit the change to both action and tile props.

@brittneytewks brittneytewks removed the design Issues that need design consultation prior to development. label Aug 24, 2023
@ashetland ashetland added the figma changes Issues that require additions or updates to the Figma UI Kit where no `design` label exists label Sep 7, 2023
@brittneytewks
Copy link

Design has confirmed this is good to go, makes sense to proceed forward

@geospatialem geospatialem added estimate - 2 Small fix or update, may require 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 Sep 7, 2023
@geospatialem geospatialem removed the needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. label Sep 13, 2023
@brittneytewks brittneytewks added the low risk Issues with low risk for consideration in low risk milestones label Sep 21, 2023
@brittneytewks brittneytewks added this to the Dev Backlog milestone Jan 17, 2024
@driskull driskull self-assigned this Jul 24, 2024
@driskull driskull added 2 - in development Issues that are actively being worked on. and removed 0 - new New issues that need assignment. labels Jul 24, 2024
driskull added a commit that referenced this issue Jul 29, 2024
**Related Issue:** #3859

## Summary

- deprecates the compact property
@driskull driskull 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 Jul 29, 2024
Copy link
Contributor

Installed and assigned for verification.

@github-actions github-actions bot assigned DitwanP and unassigned driskull Jul 29, 2024
@DitwanP
Copy link
Contributor

DitwanP commented Jul 29, 2024

🍡 Verified

@DitwanP DitwanP closed this as completed Jul 29, 2024
@DitwanP DitwanP added 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 Jul 29, 2024
@AdelheidF
Copy link

AdelheidF commented Jul 29, 2024

So compact goes away and is not being replaced with anything else?

with compact:
image image

without compact:
image image

@driskull
Copy link
Member

I think we will likely have a CSS variable added to adjust padding. Is that correct @alisonailea?

@macandcheese
Copy link
Contributor

You could use a transparent calcite-button there, which renders as a square without text present.

@alisonailea
Copy link
Contributor

I think we will likely have a CSS variable added to adjust padding. Is that correct @alisonailea?

We aren't currently making spacing variables unless it's needed. @AdelheidF would you please create a ticket?

@AdelheidF
Copy link

AdelheidF commented Jul 29, 2024

I guess arcgis-button works. To me action would make more sense though (with some spacing variables).

image

@macandcheese
Copy link
Contributor

macandcheese commented Jul 29, 2024

I think we want to see button used more when it is a standalone element not contained within a parent and positioned to the extent (in a bar, panel area, etc.). You could use margin-inline-start: auto on the above (or, some other css) to achieve the spacing you had previously. The Card Group also has some built in selection that could be explored by your designers.

cc @ashetland @SkyeSeitz for design 👀

calcite-admin pushed a commit that referenced this issue Jul 30, 2024
**Related Issue:** #3859

## Summary

- deprecates the compact property
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. enhancement Issues tied to a new feature or request. estimate - 2 Small fix or update, may require updates to tests. future breaking change Issues and pull requests with deprecation(s), requires a breaking change in a future milestone. low risk Issues with low risk for consideration in low risk milestones p - low Issue is non core or affecting less that 10% of people using the library
Projects
None yet
Development

No branches or pull requests

9 participants