Skip to content

Conversation

@jacoblogan
Copy link
Contributor

Description of changes

Add new design tokens to collection and searchfield primitives. Previously these two primitives had no design tokens so they couldn't be targeted for theming. This pull request adds a few basic tokens

Checklist

  • PR description included
  • yarn test passes
  • Tests are updated
  • No side effects or sideEffects field updated
  • Relevant documentation is changed or added (and PR referenced)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@changeset-bot
Copy link

changeset-bot bot commented Jun 2, 2022

🦋 Changeset detected

Latest commit: 01a1a67

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@aws-amplify/ui-react Patch
@aws-amplify/ui Minor
@aws-amplify/ui-vue Patch
@aws-amplify/ui-angular Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@jacoblogan
Copy link
Contributor Author

The theme example for the collection primitive will be added to the docs as part of #1947 once these tokens have been merged in.

@jacoblogan jacoblogan temporarily deployed to ci June 2, 2022 18:20 Inactive
@jacoblogan jacoblogan temporarily deployed to ci June 2, 2022 18:20 Inactive
@jacoblogan jacoblogan temporarily deployed to ci June 2, 2022 18:20 Inactive
@jacoblogan jacoblogan temporarily deployed to ci June 2, 2022 18:20 Inactive
Copy link
Contributor

@dbanksdesign dbanksdesign left a comment

Choose a reason for hiding this comment

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

Just a few minor comments/questions, but overall looks good!

@lgtm-com
Copy link

lgtm-com bot commented Jun 3, 2022

This pull request introduces 1 alert when merging 3411db7 into b1f9e53 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@jacoblogan jacoblogan temporarily deployed to ci June 3, 2022 22:52 Inactive
@jacoblogan jacoblogan temporarily deployed to ci June 3, 2022 22:52 Inactive
@jacoblogan jacoblogan temporarily deployed to ci June 3, 2022 22:52 Inactive
@jacoblogan jacoblogan temporarily deployed to ci June 3, 2022 22:52 Inactive
@jacoblogan jacoblogan temporarily deployed to ci June 4, 2022 00:13 Inactive
@jacoblogan jacoblogan temporarily deployed to ci June 4, 2022 00:13 Inactive
@jacoblogan jacoblogan temporarily deployed to ci June 4, 2022 00:13 Inactive
@jacoblogan jacoblogan temporarily deployed to ci June 4, 2022 00:13 Inactive
Copy link
Contributor

@zchenwei zchenwei left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@dbanksdesign dbanksdesign left a comment

Choose a reason for hiding this comment

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

Conditional approval, I think just the example needs a one line update

components: {
searchfield: {
color: { value: 'red' },
search: {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be button: { right? Looking at the searchField.ts file below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks that has been updated to button

Copy link
Contributor

@reesscot reesscot left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for fixing this!

@jacoblogan jacoblogan temporarily deployed to ci June 6, 2022 21:46 Inactive
@jacoblogan jacoblogan temporarily deployed to ci June 6, 2022 21:46 Inactive
@jacoblogan jacoblogan temporarily deployed to ci June 6, 2022 21:46 Inactive
@jacoblogan jacoblogan temporarily deployed to ci June 6, 2022 21:46 Inactive
@jacoblogan jacoblogan merged commit 6c267aa into main Jun 6, 2022
@jacoblogan jacoblogan deleted the add-design-tokens branch June 6, 2022 22:11
@github-actions github-actions bot mentioned this pull request Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants