feat(logo-grid): add logoCount and logoRatio props #8905

andy-blum · 2022-05-24T00:51:36Z

Related Ticket(s)

Description

Adds logoCount and logoRatio props to DDSLogoGrid

logoCount takes an integer to use as a number of columns for the grid. The stylesheet will constrain that value to between 2 and 4, inclusively.

logoRatio takes a string matching the format <integer><separator><integer> and sets a custom CSS property in an inline style to use in logo-grid-item elements as an aspect ratio.

Changelog

New

Added logoCount and logoRatio props to DDSLogoGrid

ibmdotcom-bot · 2022-05-24T01:10:31Z

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

Built with commit: ab68348b635fd68e2b4ffe44b8be084de8d1a496

ibmdotcom-bot · 2022-05-24T01:16:06Z

Deploy preview created for package NextJS Test Application:
https://ibmdotcom-nextjs-test-upstream.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/8905/index.html

Built with commit: ab68348b635fd68e2b4ffe44b8be084de8d1a496

ibmdotcom-bot · 2022-05-24T01:18:39Z

Deploy preview created for package Web Components HTML Test Application:
https://ibmdotcom-web-components-html-test-upstream.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/8905/index.html

Built with commit: ab68348b635fd68e2b4ffe44b8be084de8d1a496

ibmdotcom-bot · 2022-05-24T01:19:29Z

Deploy preview created for package Web Components (React wrapper):
https://ibmdotcom-web-components-react.s3-web.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/8905/index.html

Built with commit: ab68348b635fd68e2b4ffe44b8be084de8d1a496

ibmdotcom-bot · 2022-05-24T01:21:22Z

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

Built with commit: ab68348b635fd68e2b4ffe44b8be084de8d1a496

oliviaflory · 2022-05-24T20:26:12Z

Thanks for this @andy-blum!

1. Ratio in heading title

Storybook has been having some trouble with updating content with the knobs lately. The 4:3 is remaining when we change the ratio unless you open the story in its own tab, see the example of 2:1 but still says "4:3". I'd recommend removing the "4:3" for now and we could add it back when we figure out that weird storybook bug.

2. Red border around images

Is this temporary just for testing? Our storybook demos usually show components as intended to be used, and I'd love to hide the red border so not to cause any confusion.

3. Remove `2` option within Column count

I don't believe @jwitkowski79 and I discussed a 2 up logo, so we'd like to remove that option and keep it at the 3 and 4 options.
In the specs Jen provided I do see a 2 column minimum outlined, but I believe that means a 2 column minimum width for the logos. Could you confirm @jwitkowski79 ?

4. Medium breakpoint should have 4 logos across a row, not 3

The specs for the 4 column count option shows the logos spanning 4 across medium breakpoint.

5. Small breakpoint should have 2 logos across a row, not 1

The specs for the 4 column count option shows the logos spanning 2 across medium breakpoint.

6. Space between CTA and logos

Looks like somewhere we lost the 48px between the logo and CTA. We don't have to fix here, but I think that's what Jen is expecting eventually!

andy-blum · 2022-05-25T14:38:56Z

@oliviaflory @jwitkowski79

Can you confirm this is the proper columns-per-breakpoint configuration?

	logoCount: 3	logoCount: 4
@sm	1 column	2 columns.
@md	2 columns	4 columns
@lg	3 columns	4 columns

…for-ibm-dotcom into feat/grid-logo

jwitkowski79 · 2022-05-25T15:37:07Z

@oliviaflory @andy-blum Olivia is correct in saying that the logos should take up a 2 column count, and 2up logo variation needs to be removed.

Below is the proper columns-per-breakpoint configuration based on the design documentation:`

	logoCount: 3	logoCount: 4
@sm	2 columns	2 columns
@md	2 columns	4 columns
@lg	3 columns	4 columns

` Also, not sure if this is in another upcoming ticket but we'd love to have the content block (not just the title) display above the the logo grid as seen in the design specs

andy-blum · 2022-05-25T18:15:14Z

packages/utilities/src/utilities/parseAspectRatio/parseAspectRatio.js

+ * @returns {Array} [widthInt, heightInt]
+ */
+
+function parseAspectRatio(aspectRatioString) {


@proeung @kennylam adding this utility as we'll utilize it in #8849

packages/styles/scss/components/logo-grid/_logo-grid.scss

packages/utilities/src/utilities/parseAspectRatio/parseAspectRatio.js

kennylam

Looks good @andy-blum, just a few observations.

kennylam · 2022-05-31T12:29:18Z

@andy-blum I think you only need to adjust the e2e tests. https://github.com/carbon-design-system/carbon-for-ibm-dotcom/runs/6631472191?check_suite_focus=true#step:8:2339

oliviaflory · 2022-06-01T19:33:28Z

@andy-blum Thank you for the updates!

Small thing on the storybook knobs:

1. Column count: Can we reduce to two options?
Because there isn't a difference between Default and Three, I'd love to go with Three (as the default) and Four
But if that would break anything in terms of renaming we can go with Default and Four

2. Column count: Can we reduce to three options?
Because there isn't a difference between Default and 4:3 I'd love to go with 4:3, 16:9 and 2:1
But if that would break anything in terms of renaming we can go with Default, 16:9 and 2:1

oliviaflory

Thanks @andy-blum !

proeung

A bit of nitpicking, but I think it'll be good to update the markup of the <dds-logo-grid> CodeSandbox examples (ES & CDN) and the "Docs" tab to include these new props (logo-count and logo-ratio) so that adopters are aware of their options. You might need to double-check the e2e testing to ensure that nothing is breaking.

proeung

Looks good! Thanks for addressing my feedback.

@jwitkowski79 Can you take a look at this PR and give us a thumbs up before we mark this one as ready for merge?

jwitkowski79

@andy-blum @proeung This looks good!

feat(logo-grid): add logoCount and logoRatio props

8f9b770

andy-blum requested review from kennylam, proeung and oliviaflory May 24, 2022 00:51

andy-blum requested a review from a team as a code owner May 24, 2022 00:51

oliviaflory requested a review from jwitkowski79 May 24, 2022 15:27

oliviaflory added the Needs design approval PRs on feature requests and new components have to get design approval before merge. label May 24, 2022

Merge branch 'main' into feat/grid-logo

940f47d

proeung added package: web components Work necessary for the IBM.com Library web components package adopter: AEM used when component or pattern will be used by this adopter 👀 eyes needed labels May 24, 2022

oliviaflory mentioned this pull request May 24, 2022

[PR Reviews]: Olivia - Sprint 11 #8904

Closed

andy-blum added 7 commits May 25, 2022 10:42

feat(logo-grid): remove ratio in heading title

61e3cd3

feat(logo-grid): remove red debugging outline

31b4fb9

feat(logo-grid): remove 2-column option, update storybook knob config

bcf87d9

feat(logo-grid): update CTA spacing

b96c73c

feat(logo-grid): update snapshot

c552aab

Merge branch 'feat/grid-logo' of https://github.com/andy-blum/carbon-…

d6fcac5

…for-ibm-dotcom into feat/grid-logo

Merge branch 'main' into feat/grid-logo

fdf9b35

andy-blum added 2 commits May 25, 2022 12:39

feat(logo-grid): update grid responsive layout

fe2444b

feat(logo-grid): add parseAspectRatio utility

61e5a16

andy-blum commented May 25, 2022

View reviewed changes

kennylam reviewed May 26, 2022

View reviewed changes

packages/styles/scss/components/logo-grid/_logo-grid.scss Outdated Show resolved Hide resolved

kennylam reviewed May 26, 2022

View reviewed changes

packages/utilities/src/utilities/parseAspectRatio/parseAspectRatio.js Outdated Show resolved Hide resolved

kennylam reviewed May 26, 2022

View reviewed changes

proeung added the 🛠️ fix needed label May 26, 2022

andy-blum added 3 commits May 27, 2022 16:21

feat(logo-grid): narrow stylelint-disable scope

6601550

feat(logo-grid): fix typechecking in parseAspectRatio.js

fe41a54

Merge branch 'main' into feat/grid-logo

4993939

andy-blum added 2 commits May 31, 2022 10:28

feat(logo-grid): check for logoRatio before parsing attempt

1dcb461

Merge branch 'main' into feat/grid-logo

3c4ca8f

andy-blum removed the 🛠️ fix needed label May 31, 2022

feat(logo-grid): fix broken snapshots

a46042b

andy-blum added 2 commits June 2, 2022 09:27

feat(logo-grid): update storybook knobs

9b923d9

Merge branch 'main' into feat/grid-logo

6178a4e

oliviaflory approved these changes Jun 2, 2022

View reviewed changes

proeung reviewed Jun 2, 2022

View reviewed changes

oliviaflory removed the Needs design approval PRs on feature requests and new components have to get design approval before merge. label Jun 2, 2022

proeung added the 🛠️ fix needed label Jun 3, 2022

feat(logo-grid): add new props to codesandbox examples

faffd09

proeung approved these changes Jun 6, 2022

View reviewed changes

proeung added Needs design approval PRs on feature requests and new components have to get design approval before merge. and removed 👀 eyes needed 🛠️ fix needed labels Jun 6, 2022

jwitkowski79 approved these changes Jun 6, 2022

View reviewed changes

proeung added Ready to merge Label for the pull requests that are ready to merge and removed Needs design approval PRs on feature requests and new components have to get design approval before merge. labels Jun 6, 2022

Merge branch 'main' into feat/grid-logo

ab68348

kodiakhq bot merged commit c519b06 into carbon-design-system:main Jun 6, 2022

andy-blum deleted the feat/grid-logo branch June 7, 2022 13:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(logo-grid): add logoCount and logoRatio props #8905

feat(logo-grid): add logoCount and logoRatio props #8905

andy-blum commented May 24, 2022

ibmdotcom-bot commented May 24, 2022 •

edited

Loading

ibmdotcom-bot commented May 24, 2022 •

edited

Loading

ibmdotcom-bot commented May 24, 2022 •

edited

Loading

ibmdotcom-bot commented May 24, 2022 •

edited

Loading

ibmdotcom-bot commented May 24, 2022 •

edited

Loading

oliviaflory commented May 24, 2022

andy-blum commented May 25, 2022

jwitkowski79 commented May 25, 2022

andy-blum May 25, 2022 •

edited

Loading

kennylam left a comment •

edited

Loading

kennylam commented May 31, 2022

oliviaflory commented Jun 1, 2022

oliviaflory left a comment

proeung left a comment

proeung left a comment

jwitkowski79 left a comment

feat(logo-grid): add logoCount and logoRatio props #8905

feat(logo-grid): add logoCount and logoRatio props #8905

Conversation

andy-blum commented May 24, 2022

Related Ticket(s)

Description

Changelog

ibmdotcom-bot commented May 24, 2022 • edited Loading

ibmdotcom-bot commented May 24, 2022 • edited Loading

ibmdotcom-bot commented May 24, 2022 • edited Loading

ibmdotcom-bot commented May 24, 2022 • edited Loading

ibmdotcom-bot commented May 24, 2022 • edited Loading

oliviaflory commented May 24, 2022

1. Ratio in heading title

2. Red border around images

3. Remove 2 option within Column count

4. Medium breakpoint should have 4 logos across a row, not 3

5. Small breakpoint should have 2 logos across a row, not 1

6. Space between CTA and logos

andy-blum commented May 25, 2022

jwitkowski79 commented May 25, 2022

andy-blum May 25, 2022 • edited Loading

Choose a reason for hiding this comment

kennylam left a comment • edited Loading

Choose a reason for hiding this comment

kennylam commented May 31, 2022

oliviaflory commented Jun 1, 2022

oliviaflory left a comment

Choose a reason for hiding this comment

proeung left a comment

Choose a reason for hiding this comment

proeung left a comment

Choose a reason for hiding this comment

jwitkowski79 left a comment

Choose a reason for hiding this comment

ibmdotcom-bot commented May 24, 2022 •

edited

Loading

ibmdotcom-bot commented May 24, 2022 •

edited

Loading

ibmdotcom-bot commented May 24, 2022 •

edited

Loading

ibmdotcom-bot commented May 24, 2022 •

edited

Loading

ibmdotcom-bot commented May 24, 2022 •

edited

Loading

3. Remove `2` option within Column count

andy-blum May 25, 2022 •

edited

Loading

kennylam left a comment •

edited

Loading