Skip to content

feat: Add divider and set spacing on modals with the type=creation prop (feat-add-divider)#9629

Merged
sean-brydon merged 3 commits into
mainfrom
feat-add-divider
Jun 29, 2023
Merged

feat: Add divider and set spacing on modals with the type=creation prop (feat-add-divider)#9629
sean-brydon merged 3 commits into
mainfrom
feat-add-divider

Conversation

@gitstart-calcom
Copy link
Copy Markdown
Contributor

@gitstart-calcom gitstart-calcom commented Jun 19, 2023

What does this PR do?

  • Add the prop customDividerClassName to customize the CSS Divider if needed
  • Add a divider on all modals with type="creation" prop, following the figma design
  • Some type="creation" modals seem not to be this type, we pushed all changes with screenshots, if you want us to revert the changes on some modals we will revert

image

  • Inline Embed:
    image

  • Enable Two-Factor Authentication:
    image

-Enable Two-Factor Authentication (step 2):
image

-Enable Two-Factor Authentication (step 3):
image

  • Disable Two-Factor Authentication:
    image

  • Delete Account:
    image

  • Update Timezone:
    image

  • Duplicate Event Type:
    image

  • Add a New Form:
    image

  • Change Team Member Role:
    image

  • Impersonate:
    image

  • Bulk Update Event Types:
    image

  • Connecting with MS Teams:
    image

  • Set a Default App Link:
    image

  • OIDC configuration:
    image

  • SAML Configuration:
    image

  • How to use booking questions as variables?
    image
    image

  • Invite Link Settings:
    image

  • Add Action:
    image

  • Edit Keys:
    245192220-c669e6fe-01ba-4a66-9b03-96ab4e8f1809

  • Create an Api Key:
    image

  • API key created successfully:
    image

  • Invite Team Member:
    image

  • Add a Question:
    image

  • Add a new event type:
    image

@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 19, 2023

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

Name Status Preview Comments Updated (UTC)
api ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 26, 2023 10:47pm
cal ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 26, 2023 10:47pm
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 26, 2023 10:47pm
web-staging ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 26, 2023 10:47pm

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 19, 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 Jun 19, 2023

Current Playwright Test Results Summary

✅ 120 Passing - ⚠️ 2 Flaky

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

(Last updated on 06/26/2023 10:45:13pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: a7588ca

Started: 06/26/2023 10:43:21pm UTC

⚠️ Flakes

📄   apps/web/playwright/booking-pages.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
pro user -- old-booker can reschedule a booking
Retry 1Initial Attempt
3.37% (11) 11 / 326 runs
failed over last 7 days
3.99% (13) 13 / 326 runs
flaked over last 7 days

📄   packages/app-store/routing-forms/playwright/tests/basic.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Routing Forms Seeded Routing Form Routing Link - Reporting and CSV Download
Retry 2Retry 1Initial Attempt
6.05% (13) 13 / 215 runs
failed over last 7 days
33.95% (73) 73 / 215 runs
flaked over last 7 days

View Detailed Build Results


<DialogFooter
className="mt-8 flex flex-row-reverse gap-x-2"
showDivider
customDividerClassNames="w-2/3">
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.

In this specific case we had to implement this to the divider fill only the desired component space:
Without this prop:
244967944-bda8f054-f3da-4c30-b075-7d279a477396

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The divider should not cross into the gray part on the left ideally

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.

Ok we did this, the screenshot above is just to show why we need this prop we added 😄

<div>
<div className="flex">
<code className="bg-subtle text-default mb-2 w-full truncate rounded-md rounded-r-none py-[6px] pl-2 pr-2 align-middle font-mono">
<code className="bg-subtle text-default w-full truncate rounded-md rounded-r-none py-[6px] pl-2 pr-2 align-middle font-mono">
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.

The "mb-2" className was removed to fix this wrong input size
245198211-6b727cf9-5bc0-410e-bcf7-865918194c89

@PeerRich
Copy link
Copy Markdown
Member

what is the original issue? can you link it by commenting "fixes #number"

@ciaranha
Copy link
Copy Markdown
Member

Overall this looks great. The only thing that looks off to me is the spacing above and below the body.. ie how the body content is spaced from the header and the footer.

CleanShot 2023-06-20 at 13 55 49@2x

@ciaranha
Copy link
Copy Markdown
Member

ciaranha commented Jun 20, 2023

The following modals have Inter as the header instead of Cal Sans

  • Change Team Member Role:
  • Add a New Form

The following modals have the primary & secondary buttons order reversed (primary should be on the right):

  • Enable Two-Factor Authentication (all steps)
  • Disable Two-Factor Authentication
  • Delete Account
  • Change Team Member Role
  • Add a New Form

In designs, the button that sits alongside the primary button always use the minimal style. It appears here we're using secondary for Cancel and minimal for Close. I think we can just make those the same but no strong feelings on that one.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 26, 2023

Thank you for following the naming conventions! 🙏

@gitstart-calcom
Copy link
Copy Markdown
Contributor Author

Hi @Jaibles we made the changes here, please take a look 😄

@gitstart-calcom gitstart-calcom removed the ❗️ migrations contains migration files label Jun 29, 2023
Copy link
Copy Markdown
Member

@sean-brydon sean-brydon left a comment

Choose a reason for hiding this comment

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

I've double checked against @Jaibles comments and not noticed anything wrong. Thanks @gitstart-calcom will approve and if we notice anything else will create a issue to follow up.

Great work

@sean-brydon sean-brydon merged commit e731c33 into main Jun 29, 2023
@sean-brydon sean-brydon deleted the feat-add-divider branch June 29, 2023 07:47
@sean-brydon
Copy link
Copy Markdown
Member

@sean-brydon merged this pull request with Graphite.

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

Labels

Low priority Created by Linear-GitHub Sync

Projects

No open projects
Status: No status

Development

Successfully merging this pull request may close these issues.

5 participants