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

Card design feedback round 3 #428

Merged
merged 3 commits into from
Mar 10, 2021
Merged

Card design feedback round 3 #428

merged 3 commits into from
Mar 10, 2021

Conversation

davidmenendez
Copy link
Contributor

Contributes to #380

address design feedback for #380 (comment)

@davidmenendez davidmenendez requested a review from a team as a code owner March 10, 2021 19:16
@netlify
Copy link

netlify bot commented Mar 10, 2021

Deploy preview for ibm-cloud-cognitive ready!

Built with commit 0004ce9

https://deploy-preview-428--ibm-cloud-cognitive.netlify.app

Copy link
Contributor

@dcwarwick dcwarwick left a comment

Choose a reason for hiding this comment

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

I'm not quite sure what those size options are meant to show. But I see that this is exactly the set of options the designer has asked for, so happy to approve :-)

@davidmenendez
Copy link
Contributor Author

@dcwarwick those correspond to carbon grid column sizes https://www.carbondesignsystem.com/guidelines/2x-grid/implementation#responsive-options in the story context it's a way to display the card according to a certain column size

@dcwarwick
Copy link
Contributor

@davidmenendez Yes, but they seem a curious selection of options to me. sm-4, md-8 and max-16 will all do exactly the same thing, although theoretically responding to different breakpoints. lg-12 suddenly switches to filling 3/4 of the width rather than the full width. Going forward we might want to consider what the options for the stories need to be. But right now these are the four sizes the designer asked for so whatevs it's fine by me :-)

@davidmenendez
Copy link
Contributor Author

oh yes haha i agree. but that's what design has request 🤷

@davidmenendez davidmenendez merged commit c167a11 into carbon-design-system:master Mar 10, 2021
@davidmenendez davidmenendez deleted the card-feedback3 branch March 10, 2021 22:53
lee-chase added a commit that referenced this pull request Mar 11, 2021
* fix(EmptyState): illustrations should render correctly now

* fix(EmptyState): add dynamic import for empty state illustrations

* fix(EmptyState): use global package prefix

* fix: refactor how we implement empty state illustrations

* fix: notifications empty state test

* fix(EmptyStates): refactor empty state to not bundle every svg

* fix(EmptyStates): import sass file from new directory, fix classname

* fix: remove spurious scrollbar when available height is very small (#392)

* chore(release): publish [skip ci]

 - @carbon/[email protected]
 - @carbon/[email protected]

* fix: card design revisions v1 (#396)

* fix: card design revisions v1

* fix: card story aspect ratio update

Co-authored-by: David Menendez <[email protected]>

* chore(release): publish [skip ci]

 - @carbon/[email protected]
 - @carbon/[email protected]

* fix(SidePanel): remove slideIn prop from storybook, refactor classnames (#400)

Co-authored-by: Matt Gallo <[email protected]>

* chore(release): publish [skip ci]

 - @carbon/[email protected]
 - @carbon/[email protected]

* Remove AboutModal theme prop and respond to Carbon theme (#389)

* fix: remove AboutModal theme prop and respond to Carbon theme

* fix: add theme override stories

* chore(release): publish [skip ci]

 - @carbon/[email protected]
 - @carbon/[email protected]
 - @carbon/[email protected]
 - @carbon/[email protected]

* fix(WebTerminal): move from experimental to canary in storybook (#406)

Co-authored-by: Matt Gallo <[email protected]>

* chore(release): publish [skip ci]

 - @carbon/[email protected]
 - @carbon/[email protected]

* fix: prettier config (#402)

Co-authored-by: Lee Chase <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* fix: story sort order (#401)

* fix: story sort order

* chore: run format

Co-authored-by: Lee Chase <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* chore(release): publish [skip ci]

 - @carbon/[email protected]
 - @carbon/[email protected]
 - @carbon/[email protected]
 - @carbon/[email protected]

* fix(Notifications): add props for label strings and fix classname usage (#398)

* fix(Notifications): add props for label strings and fix classname usage

* fix(Notifications): test for toggle passes now

Co-authored-by: Matt Gallo <[email protected]>

* chore(release): publish [skip ci]

 - @carbon/[email protected]
 - @carbon/[email protected]

* fix: remove console log from page header test (#410)

Co-authored-by: Lee Chase <[email protected]>

* chore(release): publish [skip ci]

 - @carbon/[email protected]
 - @carbon/[email protected]

* fix: actdionbar item tip and title title (#403)

* fix: actdionbar item tip and title title

* fix: format issue

* Update packages/experimental/src/components/PageHeader/_page-header.scss

Co-authored-by: Dave Clark <[email protected]>

* Update packages/experimental/src/components/BreadcrumbWithOverflow/BreadcrumbWithOverflow.js

Co-authored-by: Dave Clark <[email protected]>

Co-authored-by: Lee Chase <[email protected]>
Co-authored-by: Dave Clark <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* fix(PageHeader): story previews show correctly again in docs (#414)

Co-authored-by: Matt Gallo <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* chore(release): publish [skip ci]

 - @carbon/[email protected]
 - @carbon/[email protected]

* Feature flags proposal (#387)

* fix: add feature flags

* chore: shringk import a bit

* chore: update cli test file

* chore: add canary tests to component

* chore: simplify settings implementation

* chore: run yarn format

* chore: fix side panel pkgPrefix

* chroe: correct linter error

* chore: corrections

* fix: remove override settings

* chore: remove additional canary test

* chore: review move settings to top level

* Update packages/experimental/scripts/generate/templates/DISPLAY_NAME-test.js

Co-authored-by: Dave Clark <[email protected]>

* Update packages/experimental/scripts/generate/templates/DISPLAY_NAME-test.js

Co-authored-by: Dave Clark <[email protected]>

* Update packages/experimental/scripts/generate/templates/DISPLAY_NAME-test.js

Co-authored-by: Dave Clark <[email protected]>

* Update packages/experimental/scripts/generate/templates/DISPLAY_NAME-test.js

Co-authored-by: Dave Clark <[email protected]>

* Update packages/experimental/src/components/Notifications/Notifications-test.js

Co-authored-by: Dave Clark <[email protected]>

* Update packages/experimental/src/global/js/package-settings.cmn.js

Co-authored-by: Dave Clark <[email protected]>

* Update packages/experimental/src/global/js/package-settings.cmn.js

Co-authored-by: Dave Clark <[email protected]>

* Update packages/experimental/src/index-all-disabled.js

Co-authored-by: Dave Clark <[email protected]>

* Update packages/experimental/src/index-all-enabled.js

Co-authored-by: Dave Clark <[email protected]>

* Update packages/experimental/src/global/js/package-settings.js

Co-authored-by: Dave Clark <[email protected]>

* Update packages/experimental/scripts/generate/templates/DISPLAY_NAME-test.js

Co-authored-by: Dave Clark <[email protected]>

* fix: use component display name

* Update packages/experimental/src/global/js/package-settings.cmn.js

Co-authored-by: Dave Clark <[email protected]>

* Update packages/experimental/src/components/SidePanel/SidePanel-test.js

Co-authored-by: Dave Clark <[email protected]>

* Update packages/experimental/src/__tests__/index-all-disabled-test.js

Co-authored-by: Dave Clark <[email protected]>

* Update packages/experimental/src/__tests__/index-all-enabled-test.js

Co-authored-by: Dave Clark <[email protected]>

* fix: cli files settings path

* Update packages/experimental/src/components/ExampleComponent/ExampleComponent-test.js

Co-authored-by: Dave Clark <[email protected]>

* Update packages/experimental/src/components/_Canary/Canary-test.js

Co-authored-by: Dave Clark <[email protected]>

* Update packages/experimental/src/components/_Canary/Canary.js

Co-authored-by: Dave Clark <[email protected]>

* Update packages/experimental/src/components/_Canary/_canary.scss

Co-authored-by: Dave Clark <[email protected]>

* Update packages/experimental/src/components/_Canary/index.js

Co-authored-by: Dave Clark <[email protected]>

* Update packages/experimental/src/disable-all.js

Co-authored-by: Dave Clark <[email protected]>

* Update packages/experimental/src/enable-all.js

Co-authored-by: Dave Clark <[email protected]>

Co-authored-by: Lee Chase <[email protected]>
Co-authored-by: Dave Clark <[email protected]>

* chore(release): publish [skip ci]

 - @carbon/[email protected]
 - @carbon/[email protected]
 - @carbon/[email protected]
 - @carbon/[email protected]

* Issue 385 (#417)

* fix: issues #385 - AboutModal release review issues

* fix: tidy up unwanted code

* fix: rename pkgPrefix to pkg.prefix

* chore: format source code to fix build

* fix: ci-check issues

* fix: remove redundant default prop values

Co-authored-by: moores2 <>

* chore(release): publish [skip ci]

 - @carbon/[email protected]
 - @carbon/[email protected]

* chore: fix build:storybook script to ensure dependencies built (#419)

* fix: canary scss and settings imports (#418)

Co-authored-by: Lee Chase <[email protected]>

* chore(release): publish [skip ci]

 - @carbon/[email protected]
 - @carbon/[email protected]
 - @carbon/[email protected]
 - @carbon/[email protected]

* feat(WebTerminal): add entrance/exit animation to panel (#412)

* feat(WebTerminal): add entrance/exit animation to panel

* fix(WebTerminal): remove commented out line

Co-authored-by: Matt Gallo <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* chore(release): publish [skip ci]

 - @carbon/[email protected]
 - @carbon/[email protected]
 - @carbon/[email protected]

* chore: core is no longer a dependency of experimental (#421)

* chore(release): publish [skip ci]

 - @carbon/[email protected]
 - @carbon/[email protected]
 - @carbon/[email protected]
 - @carbon/[email protected]

* chore: core is no longer a dependency of security (#423)

* chore(release): publish [skip ci]

 - @carbon/[email protected]
 - @carbon/[email protected]
 - @carbon/[email protected]

* Feature flags tidy (#425)

* fix: canary scss and settings imports

* fix: example component story usage

Co-authored-by: Lee Chase <[email protected]>

* chore(release): publish [skip ci]

 - @carbon/[email protected]
 - @carbon/[email protected]
 - @carbon/[email protected]

* Fix resizing of AboutModal (#422)

* fix: resizing of AboutModal

* fix: PR review comments

* fix: positioning of scrolling gradient at small modal heights

Co-authored-by: moores2 <>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* chore(release): publish [skip ci]

 - @carbon/[email protected]
 - @carbon/[email protected]
 - @carbon/[email protected]

* build(ci): add import test and CI check for built artifacts (#351)

* build(ci): add CI check for built artifacts

* build(ci): add test and CI check for built artifacts

* build(ci): add test and CI check for built artifacts

* build: reuse `chalk` dependency

Co-authored-by: Dave Clark <[email protected]>

* chore(release): publish [skip ci]

 - @carbon/[email protected]
 - @carbon/[email protected]
 - @carbon/[email protected]
 - @carbon/[email protected]

* fix: card feedback round 3 (#428)

Co-authored-by: David Menendez <[email protected]>

* chore(release): publish [skip ci]

 - @carbon/[email protected]
 - @carbon/[email protected]
 - @carbon/[email protected]

* fix: pkg.prefix and settings move

Co-authored-by: Matt Gallo <[email protected]>
Co-authored-by: Dave Clark <[email protected]>
Co-authored-by: kodiakhq[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: David Menendez <[email protected]>
Co-authored-by: David Menendez <[email protected]>
Co-authored-by: Lee Chase <[email protected]>
Co-authored-by: Lee Chase <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Co-authored-by: Dave Clark <[email protected]>
Co-authored-by: Simon Moore <[email protected]>
Co-authored-by: Simon Finney <[email protected]>
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.

2 participants