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

fix: update media query to work with larger phones #8116

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

DiegoCardoso
Copy link
Contributor

Description

Update the media query that sets the overlay to show on the bottom of the page to work with larger phones, such as the iPhone Pro Max.

For such devices, this current media query is not met leading to the menu overlay being opened at the top-left corner of the screen.

Before

image

After

image

Fixes #8092

Type of change

  • Bugfix
  • Feature

Update the media query that sets the overlay to show on the bottom of
the page to work with larger phones, such as the iPhone Pro Max.

For such devices, this current media query is not met leading to the
menu overlay being opened at the top-left corner of the screen.
Copy link
Member

@web-padawan web-padawan left a comment

Choose a reason for hiding this comment

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

I guess this should be then also updated in other places e.g. vaadin-select might have the same problem?

Copy link

sonarcloud bot commented Nov 8, 2024

@DiegoCardoso
Copy link
Contributor Author

DiegoCardoso commented Nov 8, 2024

I guess this should be then also updated in other places e.g. vaadin-select might have the same problem?

vaadin-select uses the same style, so it was covered. But I checked for other elements, and vaadin-date-picker should be updated as well as it was looking a bit broken after this change:

Before

image

After the change only on the menu-overlay styles

image

After updating the media query in vaadin-date-picker

image

I also updated media queries in the notification

@tomivirkki
Copy link
Member

Wouldn't incresing the thresholds just postpone the problem until someone tries with a bit larger device?

Screenshot 2024-11-08 at 16 03 48

@DiegoCardoso
Copy link
Contributor Author

Wouldn't incresing the thresholds just postpone the problem until someone tries with a bit larger device?

That's true. I wasn't able to find a value that would work for most of phones with large screens. One option would be to align the media query, in this case, with the threshold value for the media query used in the context menu to define whether a touch device is a phone or not:

_wideMediaQuery: {
type: String,
value: '(min-device-width: 750px)',
},
};
}

The problem now happens with devices that are in between the current 420px and this 750px mentioned on the code above.

@tomivirkki
Copy link
Member

Maybe it would make sense to align the context menu's behavior with the other components and instead of the current _wideMediaQuery, use the same _fullscreenMediaQuery that for example date-picker has. _touch could probably also be removed because the resulting _phone is computed from !wide && touch

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

Successfully merging this pull request may close these issues.

[menu-bar] sub-menu overlay wrongly placed in some phones
3 participants