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.

Sign up for GitHub

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

Jump to bottom

fix: footer in modals #8940

Merged
merged 21 commits into from May 30, 2023
Merged

fix: footer in modals #8940

merged 21 commits into from May 30, 2023
+23 −15

Conversation

gitstart-calcom
Copy link
Contributor

@gitstart-calcom gitstart-calcom commented May 17, 2023 •
edited

@linear
Copy link

linear bot commented May 17, 2023

CAL-1543 Fix the footer in modals

Issue arose due to an issue where users "Can't see "Save" option on a longer form/booking question"

Designs:

Modal /Documentation: https://www.figma.com/file/9MOufQNLtdkpnDucmNX10R/❖-Cal-DS?node-id=29898%3A100590&t=4ZLIcu91Nx69ib75-1

Created via Threads. See full discussion: https://threads.com/34466132294

@vercel
Copy link

vercel bot commented May 17, 2023 •
edited

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

Name Status Preview Comments Updated (UTC)
cal ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 30, 2023 3:13pm
2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
api ⬜️ Ignored (Inspect) Visit Preview May 30, 2023 3:13pm
ui ⬜️ Ignored (Inspect) Visit Preview May 30, 2023 3:13pm

@github-actions github-actions bot added ⚡ Quick Wins A collection of quick wins/quick fixes that are less than 30 minutes of work 🐛 bug Something isn't working 🧹 Improvements Improvements to existing features. Mostly UX/UI labels May 17, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 17, 2023 •
edited

📦 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

deploysentinel bot commented May 17, 2023 •
edited

No failed tests 🎉

@gitstart-calcom gitstart-calcom marked this pull request as ready for review May 17, 2023 08:07
@gitstart-calcom gitstart-calcom requested a review from a team May 17, 2023 08:07
@gitstart-calcom gitstart-calcom requested a review from a team May 17, 2023 08:07
@CarinaWolli
Copy link
Member

I think we want to add that new way of scrolling for all our modals. So we should make the changes to our Dialog component. (tagging @sean-brydon as part of the UI team)

@sean-brydon
Copy link
Member

think we want to add that new way of scrolling for all our modals. So we should make the changes to our Dialog component. (tagging @sean-brydon as part of the UI team)

Yeah we do - ideally a way of showing a clip mask when the content over flows.

I'm happy for that to be picked up in a seperate PR for it tho. I will create a new issue

@PeerRich PeerRich requested a review from sean-brydon May 17, 2023 13:31
sean-brydon
sean-brydon approved these changes May 17, 2023
View reviewed changes
Copy link
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.

Looks fine to me - @Jaibles see any issues?

@ciaranha
Copy link
Member

Looks good. Two small things:

  1. Does it look like the content is being chopped off in the wrong spot? It should run all the way to the divider.

CleanShot 2023-05-17 at 15 06 28@2x

  1. Can we reduce the padding above/below the buttons?
CleanShot 2023-05-17 at 15 07 32@2x

@gitstart-calcom
Copy link
Contributor Author

Looks good. Two small things:

  1. Does it look like the content is being chopped off in the wrong spot? It should run all the way to the divider.

CleanShot 2023-05-17 at 15 06 28@2x

  1. Can we reduce the padding above/below the buttons?
CleanShot 2023-05-17 at 15 07 32@2x

Sure, we are on it

@gitstart-calcom
Copy link
Contributor Author

Hi @Jaibles, we have implemented the requested changes. Please take a look
https://www.loom.com/share/61ccc1910f674707a8ed86aca2204513

auto-merge was automatically disabled May 29, 2023 15:51

Merge queue setting changed

@PeerRich PeerRich enabled auto-merge (squash) May 29, 2023 16:02
auto-merge was automatically disabled May 30, 2023 14:51

Merge queue setting changed

@baileypumfleet baileypumfleet added this pull request to the merge queue May 30, 2023
Merged via the queue into main with commit f72296c May 30, 2023
22 checks passed
@baileypumfleet baileypumfleet deleted the CALCOM-8414 branch May 30, 2023 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CarinaWolli CarinaWolli CarinaWolli approved these changes

@sean-brydon sean-brydon sean-brydon approved these changes

Assignees
No one assigned
Labels
🐛 bug Something isn't working 🧹 Improvements Improvements to existing features. Mostly UX/UI Low priority Created by Linear-GitHub Sync ⚡ Quick Wins A collection of quick wins/quick fixes that are less than 30 minutes of work
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[CAL-1543] Fix the footer in modals
10 participants
@gitstart-calcom @CarinaWolli @sean-brydon @ciaranha @PeerRich @baileypumfleet @gitstart @alannnc @emrysal @keithwillcode