add AppCard component #784

KhudaDad414 · 2023-09-06T14:59:26Z

Description

Added AppCard component.
Change the dark background colour of the design system.
Modified ProtocolBadge so it exports Protocol type.

Related issue(s)
fixes #749

changeset-bot · 2023-09-06T14:59:30Z

🦋 Changeset detected

Latest commit: 7bf7d7e

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

This PR includes changesets to release 1 package

Name	Type
@asyncapi/studio-ui	Minor

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

netlify · 2023-09-06T14:59:31Z

✅ Deploy Preview for modest-rosalind-098b67 ready!

Name	Link
🔨 Latest commit	`7bf7d7e`
🔍 Latest deploy log	https://app.netlify.com/sites/modest-rosalind-098b67/deploys/656865d5c0cf8e00087e4c32
😎 Deploy Preview	https://deploy-preview-784--modest-rosalind-098b67.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify · 2023-09-06T14:59:31Z

✅ Deploy Preview for asyncapi-studio-design-system ready!

Name	Link
🔨 Latest commit	`7bf7d7e`
🔍 Latest deploy log	https://app.netlify.com/sites/asyncapi-studio-design-system/deploys/656865d5594223000834aef2
😎 Deploy Preview	https://deploy-preview-784--asyncapi-studio-design-system.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

fmvilas

Lovely! Looks like in the design 🚀

Just left some comments.

packages/ui/components/AppCard.tsx

packages/ui/tailwind.config.js

KhudaDad414 · 2023-10-10T10:38:43Z

@fmvilas another round of review 🙇

fmvilas · 2023-10-10T11:01:49Z

Badges separation between client and server, and the protocols are not the same. Please make it the same 🙏

Captura de pantalla 2023-10-10 a las 13 00 51

KhudaDad414 · 2023-10-10T13:17:31Z

@fmvilas done. but left the space between client/server badge and protocol badges to keep them separated.

Amzani

LGTM

Amzani · 2023-10-24T10:32:20Z

/au

princerajpoot20

Great work @KhudaDad414! 🚀 Just a few suggestions to consider.

packages/ui/components/AppCard.tsx

princerajpoot20 · 2023-11-22T22:43:52Z

packages/ui/components/AppCard.tsx

+              {isServer && <ServiceInfoBadge info='server'/>}
+            </div>
+            }
+            {badges.map((badge, index) => (<ServiceInfoBadge info={badge} key={index} />))}


Suggested change

{badges.map((badge, index) => (<ServiceInfoBadge info={badge} key={index} />))}

{Array.from(new Set(badges))

.filter(badge => validBadges.includes(badge))

.map((badge, index) => (<ServiceInfoBadge info={badge} key={`${badge}-${index}`} />))}

I noticed that invalid badges leave a blank space (reserved for that invalid badge).
What do you think about adding a check for this also? If a badge is not valid, we could simply ignore it instead of reserving blank space.

For this, we need to add an export of the list of valid badges from ServiceInfoBadge

export const validBadges: Info[] = ['http', 'kafka', 'websocket', 'amqp', 'mqtt', 'googlepubsub', 'ibmmq', 'nats', 'pulsar', 'redis', 'sns', 'sqs', 'solace', 'stomp', 'client', 'server'];

we are already marking it's type as Info[] so it will give error in Typescript. so I am more in favour of letting typescript give the error.

Sorry, I didn't get this, could you please explain that further.

@princerajpoot20 I mean Typescript doesn't allow invalid badges:

@KhudaDad414 Okay, got it. Makes sense. 👍

Co-authored-by: Prince Rajpoot <[email protected]>

fmvilas · 2023-11-24T11:15:48Z

packages/ui/components/AppCard.tsx

+              {isServer && <ServiceInfoBadge info='server'/>}
+            </div>
+            }
+            {Array.from(new Set(badges)).map((badge, index) => (<ServiceInfoBadge info={badge} key={badge + index} aria-label={`Badge for ${badge}`} />))}


I would extract this to a variable so it's easier to understand. Above the return on line 14, I would add this:

const dedupedListOfBadges = Array.from(new Set(badges)).map((badge, index) => (<ServiceInfoBadge info={badge} key={badge + index} aria-label={`Badge for ${badge}`} />))

And then in this line I would just add {dedupedListOfBadges}. The code would be much more readable IMHO.

Also, for the aria-label, this is what a screen reader would say so imagine having 4 or 5 badges. The screen reader would say: "Badge for client, Badge for server, Badge for websocket, Badge for HTTP, Badge for kafka" 😅 Badge badge badge 😂

I suggest that we improve the root element aria-label with a better explanation and would mark all these badges with aria-hidden={true}. This way we can create a meaningful sentence like "This application is both a server and a client. It uses the WebSocket, HTTP, and Kafka protocols".

Suggested change

{Array.from(new Set(badges)).map((badge, index) => (<ServiceInfoBadge info={badge} key={badge + index} aria-label={`Badge for ${badge}`} />))}

{Array.from(new Set(badges)).map((badge, index) => (<ServiceInfoBadge info={badge} key={badge + index} aria-hidden={true} />))}

Then on line 17, we change it to:

aria-label={ariaLabel}

Where ariaLabel is something like "{name}. This application is active/inactive, and it's both a server and a client. It uses the WebSocket, HTTP, and Kafka protocols."

Note we're missing the description though. I'm not sure if a screen reader would read the paragraph below because it's not focusable. We should add https://storybook.js.org/addons/addon-screen-reader to Storybook so we can properly test what our users will experience using a screen reader.

I would extract this to a variable so it's easier to understand.

Agreed, extracting the badges list makes the code cleaner.

Also, for the aria-label, this is what a screen reader would say so imagine having 4 or 5 badges. The screen reader would say: "Badge for client, Badge for server, Badge for websocket, Badge for HTTP, Badge for kafka"

I see your point, and I agree. Right now, it is not a fit for 4-5 badges.

I suggest that we improve the root element aria-label with a better explanation and would mark all these badges with aria-hidden={true}. This way we can create a meaningful sentence like "This application is both a server and a client. It uses the WebSocket, HTTP, and Kafka protocols".
We should add https://storybook.js.org/addons/addon-screen-reader to Storybook so we can properly test what our users will experience using a screen reader.

Sounds perfect.

packages/ui/components/AppCard.tsx

Co-authored-by: Fran Méndez <[email protected]>

sonarqubecloud · 2023-11-30T10:37:46Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

No Coverage information
0.0% Duplication

fmvilas

LGTM 👍

KhudaDad414 · 2023-11-30T11:02:32Z

/rtm

KhudaDad414 requested review from fmvilas, boyney123, mcturco, magicmatatjahu and asyncapi-bot-eve as code owners September 6, 2023 14:59

fmvilas requested changes Sep 6, 2023

View reviewed changes

packages/ui/components/AppCard.tsx Outdated Show resolved Hide resolved

packages/ui/tailwind.config.js Outdated Show resolved Hide resolved

KhudaDad414 requested a review from fmvilas October 5, 2023 15:27

Amzani previously approved these changes Oct 16, 2023

View reviewed changes

asyncapi-bot added the autoupdate label Oct 24, 2023

KhudaDad414 and others added 8 commits October 24, 2023 12:07

change default dark color to our backgournd

994ddcb

modify ProtocolBadge so the protocol can be used by other components.

dd909f3

add AppCard component

4acfea5

add changeset

d08a00e

rename protocols to badges

19c88d2

Update fluffy-plums-mix.md

a1a09fa

add isClient

92bce22

remove space

a590b21

KhudaDad414 force-pushed the app-component branch from f0e8194 to a590b21 Compare October 24, 2023 11:09

KhudaDad414 dismissed Amzani’s stale review via 99e267e October 24, 2023 11:16

KhudaDad414 force-pushed the app-component branch from 99e267e to a590b21 Compare October 24, 2023 11:19

KhudaDad414 and others added 3 commits October 24, 2023 13:40

Merge branch 'master' into app-component

1fc1384

Merge branch 'master' into app-component

9e37b30

Merge branch 'master' into app-component

0d06f42

KhudaDad414 requested a review from Amzani November 2, 2023 15:47

princerajpoot20 requested changes Nov 22, 2023

View reviewed changes

KhudaDad414 and others added 4 commits November 23, 2023 16:28

Update packages/ui/components/AppCard.tsx

3c35237

Co-authored-by: Prince Rajpoot <[email protected]>

Update packages/ui/components/AppCard.tsx

ee06d28

Co-authored-by: Prince Rajpoot <[email protected]>

Update packages/ui/components/AppCard.tsx

7fbc518

Co-authored-by: Prince Rajpoot <[email protected]>

Merge branch 'master' into app-component

bddc36b

fmvilas requested changes Nov 24, 2023

View reviewed changes

KhudaDad414 and others added 2 commits November 27, 2023 13:07

fix aria label

2d62783

Merge branch 'master' into app-component

0482955

KhudaDad414 requested review from fmvilas and princerajpoot20 November 27, 2023 13:12

Amzani mentioned this pull request Nov 28, 2023

chore: add princerajpoot20 as code owner for ds and ui #862

Merged

fmvilas requested changes Nov 29, 2023

View reviewed changes

packages/ui/components/AppCard.tsx Outdated Show resolved Hide resolved

KhudaDad414 and others added 2 commits November 30, 2023 10:36

Update packages/ui/components/AppCard.tsx

6e17d58

Co-authored-by: Fran Méndez <[email protected]>

Merge branch 'master' into app-component

7bf7d7e

KhudaDad414 requested a review from fmvilas November 30, 2023 10:37

fmvilas approved these changes Nov 30, 2023

View reviewed changes

asyncapi-bot added the ready-to-merge label Nov 30, 2023

asyncapi-bot merged commit 37bdfa0 into asyncapi:master Nov 30, 2023
20 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add AppCard component #784

add AppCard component #784

KhudaDad414 commented Sep 6, 2023 •

edited

Loading

changeset-bot bot commented Sep 6, 2023 •

edited

Loading

netlify bot commented Sep 6, 2023 •

edited

Loading

netlify bot commented Sep 6, 2023 •

edited

Loading

fmvilas left a comment

KhudaDad414 commented Oct 10, 2023

fmvilas commented Oct 10, 2023

KhudaDad414 commented Oct 10, 2023

Amzani left a comment

Amzani commented Oct 24, 2023

princerajpoot20 left a comment •

edited

Loading

princerajpoot20 Nov 22, 2023 •

edited

Loading

KhudaDad414 Nov 27, 2023

princerajpoot20 Nov 28, 2023

KhudaDad414 Nov 29, 2023

princerajpoot20 Nov 29, 2023

fmvilas Nov 24, 2023

fmvilas Nov 24, 2023

princerajpoot20 Nov 25, 2023

sonarqubecloud bot commented Nov 30, 2023

fmvilas left a comment

KhudaDad414 commented Nov 30, 2023

	{Array.from(new Set(badges)).map((badge, index) => (<ServiceInfoBadge info={badge} key={badge + index} aria-label={`Badge for ${badge}`} />))}
	{Array.from(new Set(badges)).map((badge, index) => (<ServiceInfoBadge info={badge} key={badge + index} aria-hidden={true} />))}

add AppCard component #784

add AppCard component #784

Conversation

KhudaDad414 commented Sep 6, 2023 • edited Loading

changeset-bot bot commented Sep 6, 2023 • edited Loading

🦋 Changeset detected

netlify bot commented Sep 6, 2023 • edited Loading

✅ Deploy Preview for modest-rosalind-098b67 ready!

netlify bot commented Sep 6, 2023 • edited Loading

✅ Deploy Preview for asyncapi-studio-design-system ready!

fmvilas left a comment

Choose a reason for hiding this comment

KhudaDad414 commented Oct 10, 2023

fmvilas commented Oct 10, 2023

KhudaDad414 commented Oct 10, 2023

Amzani left a comment

Choose a reason for hiding this comment

Amzani commented Oct 24, 2023

princerajpoot20 left a comment • edited Loading

Choose a reason for hiding this comment

princerajpoot20 Nov 22, 2023 • edited Loading

Choose a reason for hiding this comment

KhudaDad414 Nov 27, 2023

Choose a reason for hiding this comment

princerajpoot20 Nov 28, 2023

Choose a reason for hiding this comment

KhudaDad414 Nov 29, 2023

Choose a reason for hiding this comment

princerajpoot20 Nov 29, 2023

Choose a reason for hiding this comment

fmvilas Nov 24, 2023

Choose a reason for hiding this comment

fmvilas Nov 24, 2023

Choose a reason for hiding this comment

princerajpoot20 Nov 25, 2023

Choose a reason for hiding this comment

sonarqubecloud bot commented Nov 30, 2023

fmvilas left a comment

Choose a reason for hiding this comment

KhudaDad414 commented Nov 30, 2023

KhudaDad414 commented Sep 6, 2023 •

edited

Loading

changeset-bot bot commented Sep 6, 2023 •

edited

Loading

netlify bot commented Sep 6, 2023 •

edited

Loading

netlify bot commented Sep 6, 2023 •

edited

Loading

princerajpoot20 left a comment •

edited

Loading

princerajpoot20 Nov 22, 2023 •

edited

Loading