Skip to content

feat: Connect component for Cal Atoms#11605

Closed
Ryukemeister wants to merge 45 commits intomainfrom
connect-component
Closed

feat: Connect component for Cal Atoms#11605
Ryukemeister wants to merge 45 commits intomainfrom
connect-component

Conversation

@Ryukemeister
Copy link
Copy Markdown
Contributor

@Ryukemeister Ryukemeister commented Sep 29, 2023

What does this PR do?

Adds a Connect button component for Cal Atoms

@vercel
Copy link
Copy Markdown

vercel Bot commented Sep 29, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ai ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 23, 2023 7:26am
api ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 23, 2023 7:26am
dev ❌ Failed (Inspect) Nov 23, 2023 7:26am
4 Ignored Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Nov 23, 2023 7:26am
cal-demo ⬜️ Ignored (Inspect) Visit Preview Nov 23, 2023 7:26am
qa ⬜️ Ignored (Inspect) Visit Preview Nov 23, 2023 7:26am
ui ⬜️ Ignored (Inspect) Visit Preview Nov 23, 2023 7:26am

@Ryukemeister Ryukemeister marked this pull request as draft September 29, 2023 06:42
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Sep 29, 2023

Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link to collect XP and win prizes!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Sep 29, 2023

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@deploysentinel
Copy link
Copy Markdown

deploysentinel Bot commented Sep 29, 2023

Current Playwright Test Results Summary

✅ 136 Passing - ⚠️ 2 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 10/06/2023 07:45:36am UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: f0febb0

Started: 10/06/2023 07:43:23am UTC

⚠️ Flakes

📄   apps/web/playwright/profile.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Teams Profile page is loaded for users in Organization
Retry 2Retry 1Initial Attempt
3.24% (6) 6 / 185 runs
failed over last 7 days
28.65% (53) 53 / 185 runs
flaked over last 7 days

📄   packages/embeds/embed-core/playwright/tests/action-based.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Popup Tests should be able to reschedule
Retry 1Initial Attempt
4.12% (10) 10 / 243 runs
failed over last 7 days
93% (226) 226 / 243 runs
flaked over last 7 days

View Detailed Build Results


Comment thread packages/atoms/connect/ConnectButton.tsx Outdated
Copy link
Copy Markdown
Contributor

@supalarry supalarry left a comment

Choose a reason for hiding this comment

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

Well done Rajiv! Left some refactor comments.

Comment thread packages/atoms/connect/ConnectButton.tsx Outdated
Comment thread packages/atoms/connect/ConnectButton.tsx
Comment thread packages/atoms/src/lib/utils.ts
Comment thread packages/atoms/connect/ConnectButton.tsx Outdated
export function ConnectButton({
buttonText,
handleClick,
onClick,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@supalarry this is the correct way to do it right?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah i think this makes sense. Can you manually test it by using the component, passing onClick to it and seeing if it is fired when the button is clicked?

Copy link
Copy Markdown
Contributor Author

@Ryukemeister Ryukemeister Oct 27, 2023

Choose a reason for hiding this comment

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

Should work IMHO but will test and let you know, great review btw!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@supalarry I added a loader for the button can you review it again and make sure it's alright?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good!

@PeerRich
Copy link
Copy Markdown
Member

PeerRich commented Nov 2, 2023

just to confirm, this has no functionality yet right? Its supposed to connect to Google Calendar, zoom, etc.

@Ryukemeister
Copy link
Copy Markdown
Contributor Author

Ryukemeister commented Nov 2, 2023

just to confirm, this has no functionality yet right? Its supposed to connect to Google Calendar, zoom, etc.

Correct, it doesn't handle any functionality. The functionality can be passed in through the onClick handler and all button does is execute the method (connect to Google Calendar, zoom, etc) that will be passed onto it in the form of props.

Comment thread packages/atoms/connect/ConnectButton.tsx Outdated
@PeerRich
Copy link
Copy Markdown
Member

PeerRich commented Nov 9, 2023

Correct, it doesn't handle any functionality. The functionality can be passed in through the onClick handler and all button does is execute the method (connect to Google Calendar, zoom, etc) that will be passed onto it in the form of props.

ideally the Connect button has logic, especially the logic to connect to something. that's the whole point of this component.

<Connect to="Google Calendar"/>

(not final props API) but somewhat like this

@Ryukemeister
Copy link
Copy Markdown
Contributor Author

Correct, it doesn't handle any functionality. The functionality can be passed in through the onClick handler and all button does is execute the method (connect to Google Calendar, zoom, etc) that will be passed onto it in the form of props.

ideally the Connect button has logic, especially the logic to connect to something. that's the whole point of this component.

<Connect to="Google Calendar"/>

(not final props API) but somewhat like this

Will keep this in mind

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 8, 2023

This PR is being marked as stale due to inactivity.

@github-actions
Copy link
Copy Markdown
Contributor

This PR is being closed due to inactivity. Please reopen if work is intended to be continued.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants