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

feat(block): add icon start/end properties (deprecate icon slot and status), add actions-end slot (deprecate control), add content-start #9535

Merged
merged 23 commits into from
Jun 17, 2024

Conversation

Elijbet
Copy link
Contributor

@Elijbet Elijbet commented Jun 6, 2024

Related Issue: #4932

Summary

  • Add icon-start and icon-end properties.
  • Deprecate status.
  • Deprecate icon in favor of icon-start.
  • Add actions-end slot and deprecate control slot.
  • Add content-start.

@github-actions github-actions bot added the enhancement Issues tied to a new feature or request. label Jun 7, 2024
@Elijbet Elijbet marked this pull request as ready for review June 7, 2024 15:07
@Elijbet Elijbet requested a review from a team as a code owner June 7, 2024 15:07
@Elijbet Elijbet added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Jun 7, 2024
Copy link
Member

@driskull driskull left a comment

Choose a reason for hiding this comment

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

👍

packages/calcite-components/src/components/block/block.tsx Outdated Show resolved Hide resolved
@benelan benelan changed the base branch from main to dev June 10, 2024 09:06
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.

Aside from a few comments, this LGTM!

Before merging, can you update the PR title to mention the new slots and to call out the deprecated ones (similar to this)?

* @slot icon - A slot for adding a leading header icon with `calcite-icon`.
* @slot control - A slot for adding a single HTML input element in a header.
* @slot actions-end - A slot for adding actionable `calcite-action` elements after the content of the component. It is recommended to use two or fewer actions.
* @slot icon - [Deprecated] A slot for adding a leading header icon with `calcite-icon`. Use `icon-start` instead.
Copy link
Member

Choose a reason for hiding this comment

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

✨🏆✨

packages/calcite-components/src/components/block/block.tsx Outdated Show resolved Hide resolved
packages/calcite-components/src/components/block/block.tsx Outdated Show resolved Hide resolved
packages/calcite-components/src/components/block/block.tsx Outdated Show resolved Hide resolved
packages/calcite-components/src/demos/block.html Outdated Show resolved Hide resolved
@Elijbet Elijbet changed the title feat(block): add icon property, update slot nomenclature feat(block): add icon start/end properties (deprecate icon slot and status), add actions-end slot (deprecate control), add content-start Jun 10, 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 Jun 11, 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 Jun 11, 2024
@Elijbet Elijbet added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Jun 11, 2024
@ashetland
Copy link

Looks great!

@macandcheese
Copy link
Contributor

macandcheese commented Jun 11, 2024

This is looking great! More on the design side - can we consider using the xxs spacing for the loading spinner? It is slightly misaligned with other Blocks (or, the same block shifts when setting loading). cc @ashetland @SkyeSeitz

Screenshot 2024-06-11 at 3 02 04 PM Screenshot 2024-06-11 at 3 00 43 PM

@ashetland
Copy link

ashetland commented Jun 11, 2024

This is looking great! More on the design side - can we consider using the xxs spacing for the loading spinner? It is slightly misaligned with other Blocks (or, the same block shifts when setting loading). cc @ashetland @SkyeSeitz

This was documented in #7315, but closed in favor of #7492. Might be best to move forward with that?

@macandcheese
Copy link
Contributor

macandcheese commented Jun 11, 2024

This is looking great! More on the design side - can we consider using the xxs spacing for the loading spinner? It is slightly misaligned with other Blocks (or, the same block shifts when setting loading). cc @ashetland @SkyeSeitz

This was documented in #7315, but closed in favor of #7492. Might be best to move forward with that?

Sure, just noticed it and would be a related adjustment now (just a token change) - the larger re-design should still happen but it is still in design.

@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 Jun 12, 2024
@ashetland
Copy link

Lookin' even better! 🚀🤘

@Elijbet Elijbet dismissed alisonailea’s stale review June 17, 2024 18:01

The changes to CSS have been addressed and approved by design and dev.

@Elijbet Elijbet merged commit 7117c6b into dev Jun 17, 2024
12 checks passed
@Elijbet Elijbet deleted the elijbet/4932-block-add-icon-prop-update-slot-nomenclature branch June 17, 2024 18:03
@github-actions github-actions bot added this to the 2024-06-25 - Jun Release milestone Jun 17, 2024
@benelan benelan mentioned this pull request Aug 28, 2024
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants