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.

Sign up for GitHub

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

Jump to bottom

feat(callout with media): web component version #4343

Merged
merged 18 commits into from
Nov 5, 2020
Merged

feat(callout with media): web component version #4343

merged 18 commits into from
Nov 5, 2020

Conversation

ariellalgilmore
Copy link
Member

Related Ticket(s)

#3747

Description

create callout with media web component

Changelog

New

  • component, tests, and stories

Changed

  • updated styles

@ariellalgilmore ariellalgilmore added hacktoberfest package: web components Work necessary for the IBM.com Library web components package labels Oct 29, 2020
@ariellalgilmore ariellalgilmore marked this pull request as ready for review October 29, 2020 16:18
@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Oct 29, 2020 •
edited
Loading

Deploy preview created for package React:
https://ibmdotcom-react.s3.us-south.cloud-object-storage.appdomain.cloud/deploy-previews/4343/index.html

Built with commit: b718a5ef3ce96f0f4a64da8c88e4dcf3ba2c27c0

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Oct 29, 2020 •
edited
Loading

Deploy preview created for package Web Components:
https://ibmdotcom-webcomponents.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/4343/index.html

Built with commit: b718a5ef3ce96f0f4a64da8c88e4dcf3ba2c27c0

@ariellalgilmore ariellalgilmore marked this pull request as draft October 29, 2020 17:43
asudoh
asudoh reviewed Oct 29, 2020
View reviewed changes
Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

Good to see the progress @ariellalgilmore! A couple of suggestions to address the ESLint error. BTW any blockers to move this out of draft?

...s/web-components/src/components/callout-with-media/__stories__/callout-with-media.stories.ts Outdated Show resolved Hide resolved
packages/web-components/src/components/callout-with-media/__tests__/callout-with-media.test.ts Outdated Show resolved Hide resolved
ariellalgilmore and others added 2 commits October 29, 2020 16:15
@ariellalgilmore @asudoh
Update packages/web-components/src/components/callout-with-media/__st…
fdb0943 
…ories__/callout-with-media.stories.ts

Co-authored-by: Akira Sudoh <[email protected]>
@ariellalgilmore @asudoh
Update packages/web-components/src/components/callout-with-media/__te…
4c7e4e2 
…sts__/callout-with-media.test.ts

Co-authored-by: Akira Sudoh <[email protected]>
@ariellalgilmore ariellalgilmore marked this pull request as ready for review October 29, 2020 23:15
asudoh
asudoh approved these changes Oct 29, 2020
View reviewed changes
Copy link
Contributor

@asudoh asudoh 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 @ariellalgilmore!

jeffchew
jeffchew requested changes Oct 30, 2020
View reviewed changes
Copy link
Member

@jeffchew jeffchew left a comment

Choose a reason for hiding this comment

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

Hi @ariellalgilmore , can you add stable selectors and prop table to the documentation?

@ariellalgilmore
Copy link
Member Author

@jeffchew updated! thank you!

@jeffchew
Copy link
Member

jeffchew commented Nov 3, 2020

@ariellalgilmore can you look to see what is happening here?

cc: @kennylam

Screen Shot 2020-11-03 at 10 30 29 AM

@ariellalgilmore
Copy link
Member Author

@jeffchew When comparing to the react version these changes seem to more closely align. Before some styles of pictogram weren't actually being applied. On the left is web components canary where for example margin-top is not actually being applied and on the right is from my PR where it is now applied causing these slight changes.
Screen Shot 2020-11-03 at 7 40 10 AM

kennylam
kennylam approved these changes Nov 5, 2020
View reviewed changes
Copy link
Member

@kennylam kennylam 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 @ariellalgilmore!

@jeffchew jeffchew added the Ready to merge Label for the pull requests that are ready to merge label Nov 5, 2020
@kodiakhq kodiakhq bot merged commit 1297c24 into carbon-design-system:master Nov 5, 2020
@ariellalgilmore ariellalgilmore deleted the feat/callout-with-media branch November 5, 2020 18:38
@jeffchew jeffchew mentioned this pull request Nov 10, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeffchew jeffchew jeffchew approved these changes

@asudoh asudoh asudoh approved these changes

@kennylam kennylam kennylam approved these changes

Assignees
No one assigned
Labels
package: web components Work necessary for the IBM.com Library web components package Ready to merge Label for the pull requests that are ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ariellalgilmore @ibmdotcom-bot @jeffchew @kennylam @asudoh @raphaelamadeu-zz