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.

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

booking page: only scroll to bottom on mobile #6453

Merged
merged 2 commits into from
Jan 13, 2023

Conversation

PeerRich
Copy link
Member

fixes #6452

previously we scrolled at any view point which causes issues on tablet if the outside container is too big when embedded.

todo:

  • refactor and move useMediaQuery from /web/lib/hooks this into packages/lib/hooks/ and fix imports

@PeerRich PeerRich linked an issue Jan 13, 2023 that may be closed by this pull request
@linear
Copy link

linear bot commented Jan 13, 2023

CAL-774 Calendar widget on iPads

Short description of the issue:

For some devices on a page with embedded iFrame the timeslots become invisible after being chosen the second time.

Environment:

  • Chrome/Safari on iPad Pro 11 2022 v16.0 Viewport 834 X 1075 px
  • Chrome/Safari on iPad Pro 12.9 2022 v16.0 Viewport 1024 x 1292 px

Steps to reproduce:

  1. Navigate to a page with embedded inline iframe from iPad (exact devices listed above)
  2. Click on one of the dates from the calendar
  3. Click on another date

Actual Result: When a date is selected a second time, the time slots component moves up and becomes invisible
Expected Result: The timeslots in iFrame should be visible

Recording:
https://user-images.githubusercontent.com/33749906/212281901-a75f0a70-a39e-494b-85ac-a12ad5e6bf5d.mp4

@vercel
Copy link

vercel bot commented Jan 13, 2023

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

Name Status Preview Updated
cal ✅ Ready (Inspect) Visit Preview Jan 13, 2023 at 10:56AM (UTC)

Comment on lines +1 to 4
// lets refactor and move this into packages/lib/hooks/
import { useState, useEffect } from "react";

const useMediaQuery = (query: string) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

@zomars can you explain me why some lib/hooks are in /apps/web and others in packages/libs/hooks?

What is the decision making process of where the hook should go?

the useMediaQuery hook seems to be reusable in all of our apps so I am surprised to find it here.

I duplicated the code now because I am not sure if we want to refactor or keep it separate

Copy link
Member Author

Choose a reason for hiding this comment

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

if we want to refactor, lets move this into packages and adjust the imports

Comment on lines +140 to +141
const isMobile = useMediaQuery("(max-width: 768px)");

Copy link
Member Author

Choose a reason for hiding this comment

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

note: i was initially looking for something that would use our tailwind config but sometimes you want to trigger this on a pixel-perfect level and not based on tailwind sm or md .. breakpoints so we'll keep this pixel based

@PeerRich PeerRich merged commit 8fc0dfb into main Jan 13, 2023
@PeerRich PeerRich deleted the 6452-cal-774-calendar-widget-on-ipads branch January 13, 2023 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CAL-774] Calendar widget on iPads
2 participants