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

Use dynamic height and width for dialogs #4567

Merged
merged 2 commits into from
May 7, 2024
Merged

Use dynamic height and width for dialogs #4567

merged 2 commits into from
May 7, 2024

Conversation

dgreif
Copy link
Member

@dgreif dgreif commented May 7, 2024

Dialog v2 in right/left position currently flows off the top of the screen on iOS devices. This is because we use 100vh, which can be larger than the visible view port. I'd like to fix this so dialogs never get cut off on iOS.

I updated both Dialogs to use 100dvh and 100dvw instead of 100vh and 100vw. These are supported on Safari 15+, which is all that we officially support on dotcom today.

Before After
A right positioned dialog with the header and footer cut off on iOS A right positioned dialog with header and footer fully in view

Changelog

New

Changed

Use dynamic view height/width for dialogs. This allows available visible space to be properly computed on iOS devices.

Removed

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

Testing & Reviewing

Merge checklist

@dgreif dgreif requested a review from a team as a code owner May 7, 2024 15:03
@dgreif dgreif requested a review from pksjce May 7, 2024 15:03
Copy link

changeset-bot bot commented May 7, 2024

🦋 Changeset detected

Latest commit: f03d1f7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented May 7, 2024

Uh oh! @dgreif, the image you shared is missing helpful alt text. Check your pull request body.

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

🤖 Beep boop! This comment was added automatically by github/accessibility-alt-text-bot.

Copy link
Contributor

github-actions bot commented May 7, 2024

size-limit report 📦

Path Size
packages/react/dist/browser.esm.js 88.07 KB (+0.04% 🔺)
packages/react/dist/browser.umd.js 88.41 KB (+0.04% 🔺)

@github-actions github-actions bot temporarily deployed to storybook-preview-4567 May 7, 2024 15:07 Inactive
@dgreif dgreif self-assigned this May 7, 2024
Copy link
Member

@siddharthkp siddharthkp left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Based on the browsers we support, this should be safe!

@dgreif dgreif added this pull request to the merge queue May 7, 2024
Merged via the queue into main with commit e39fcf8 May 7, 2024
30 checks passed
@dgreif dgreif deleted the dg/dialog-ios-height branch May 7, 2024 16:42
@primer primer bot mentioned this pull request May 7, 2024
TylerJDev pushed a commit that referenced this pull request May 7, 2024
* Use dynamic height and width for dialogs

* Update tall-forks-bathe.md

---------

Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>
JelloBagel pushed a commit that referenced this pull request May 16, 2024
* Use dynamic height and width for dialogs

* Update tall-forks-bathe.md

---------

Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>
github-merge-queue bot pushed a commit that referenced this pull request May 23, 2024
* Add new props to `FormControl.Label`

* Add conditional

* Add changeset

* Update docs json

* Add more examples

* chore(deps-dev): bump ejs from 3.1.9 to 3.1.10 (#4549)

Bumps [ejs](https://github.com/mde/ejs) from 3.1.9 to 3.1.10.
- [Release notes](https://github.com/mde/ejs/releases)
- [Commits](mde/ejs@v3.1.9...v3.1.10)

---
updated-dependencies:
- dependency-name: ejs
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* BranchName: Add style for span and add v8 tokens (#4556)

* add style for branchName as span adn add v8 tokens

* added changeset

* Update thin-ligers-turn.md

* test(vrt): update snapshots

* use not a instead of matching for span

---------

Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>
Co-authored-by: lukasoppermann <lukasoppermann@users.noreply.github.com>

* refactor(Banner): update region to use a dedicated aria-label (#4539)

* refactor(Banner): update region to use a dedicated aria-label

* chore: add changeset, update config to exclude codesandbox

* feat: add aria-label to Banner

* Update packages/react/src/Banner/Banner.test.tsx

Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com>

* Update packages/react/src/Banner/Banner.test.tsx

Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com>

* test: update test label with aria-label

---------

Co-authored-by: Josh Black <joshblack@users.noreply.github.com>
Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com>

* chore(deps-dev): bump cross-env from 7.0.2 to 7.0.3 (#4561)

Bumps [cross-env](https://github.com/kentcdodds/cross-env) from 7.0.2 to 7.0.3.
- [Release notes](https://github.com/kentcdodds/cross-env/releases)
- [Changelog](https://github.com/kentcdodds/cross-env/blob/master/CHANGELOG.md)
- [Commits](kentcdodds/cross-env@v7.0.2...v7.0.3)

---
updated-dependencies:
- dependency-name: cross-env
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps-dev): bump @babel/plugin-transform-modules-commonjs (#4562)

Bumps [@babel/plugin-transform-modules-commonjs](https://github.com/babel/babel/tree/HEAD/packages/babel-plugin-transform-modules-commonjs) from 7.23.3 to 7.24.1.
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.24.1/packages/babel-plugin-transform-modules-commonjs)

---
updated-dependencies:
- dependency-name: "@babel/plugin-transform-modules-commonjs"
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps-dev): bump jest-fail-on-console from 3.1.1 to 3.2.0 (#4563)

Bumps [jest-fail-on-console](https://github.com/ValentinH/jest-fail-on-console) from 3.1.1 to 3.2.0.
- [Release notes](https://github.com/ValentinH/jest-fail-on-console/releases)
- [Commits](ValentinH/jest-fail-on-console@v3.1.1...v3.2.0)

---
updated-dependencies:
- dependency-name: jest-fail-on-console
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* feat(FeatureFlags): broaden feature flag type to accept undefined (#4554)

* feat(FeatureFlags): loosen feature flag type to accept undefined

* chore: add changeset

* Update .changeset/grumpy-coats-worry.md

---------

Co-authored-by: Josh Black <joshblack@users.noreply.github.com>

* prevent form submit (#4551)

* deprecate title prop on ActionList.Group component on docs (#4544)

* chore: add hydro analytics to storybook (#4558)

* chore: add hydro analytics to storybook

* chore: use previewHead over managerHead

* Update build-docs

---------

Co-authored-by: Josh Black <joshblack@users.noreply.github.com>

* Revert "Revert "Add support for nested submenus to `ActionMenu`"" (#4486)

* Revert "Revert "Add support for nested submenus to `ActionMenu` (#4386)" (#4472)"

This reverts commit 82072eb.

* just want a change to trigger rebuild

---------

Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>
Co-authored-by: Pavithra Kodmad <pksjce@github.com>

* chore(deps): update typescript to 5.4.5 (#4568)

Co-authored-by: Josh Black <joshblack@users.noreply.github.com>

* Use dynamic height and width for dialogs (#4567)

* Use dynamic height and width for dialogs

* Update tall-forks-bathe.md

---------

Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>

* Make asterisk default, update story scenarios

* Update packages/react/src/FormControl/FormControl.docs.json

Co-authored-by: Owen Niblock <owenniblock@github.com>

* Update packages/react/src/internal/components/InputLabel.tsx

Co-authored-by: Owen Niblock <owenniblock@github.com>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Lukas Oppermann <lukasoppermann@github.com>
Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>
Co-authored-by: lukasoppermann <lukasoppermann@users.noreply.github.com>
Co-authored-by: Josh Black <joshblack@github.com>
Co-authored-by: Josh Black <joshblack@users.noreply.github.com>
Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com>
Co-authored-by: Armağan <broccolinisoup@github.com>
Co-authored-by: Ian Sanders <iansan5653@github.com>
Co-authored-by: Pavithra Kodmad <pksjce@github.com>
Co-authored-by: Dusty Greif <dgreif@users.noreply.github.com>
Co-authored-by: Owen Niblock <owenniblock@github.com>
khiga8 added a commit that referenced this pull request May 31, 2024
* Add new props to `FormControl.Label`

* Add conditional

* Add changeset

* Update docs json

* Add more examples

* chore(deps-dev): bump ejs from 3.1.9 to 3.1.10 (#4549)

Bumps [ejs](https://github.com/mde/ejs) from 3.1.9 to 3.1.10.
- [Release notes](https://github.com/mde/ejs/releases)
- [Commits](mde/ejs@v3.1.9...v3.1.10)

---
updated-dependencies:
- dependency-name: ejs
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* BranchName: Add style for span and add v8 tokens (#4556)

* add style for branchName as span adn add v8 tokens

* added changeset

* Update thin-ligers-turn.md

* test(vrt): update snapshots

* use not a instead of matching for span

---------

Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>
Co-authored-by: lukasoppermann <lukasoppermann@users.noreply.github.com>

* refactor(Banner): update region to use a dedicated aria-label (#4539)

* refactor(Banner): update region to use a dedicated aria-label

* chore: add changeset, update config to exclude codesandbox

* feat: add aria-label to Banner

* Update packages/react/src/Banner/Banner.test.tsx

Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com>

* Update packages/react/src/Banner/Banner.test.tsx

Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com>

* test: update test label with aria-label

---------

Co-authored-by: Josh Black <joshblack@users.noreply.github.com>
Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com>

* chore(deps-dev): bump cross-env from 7.0.2 to 7.0.3 (#4561)

Bumps [cross-env](https://github.com/kentcdodds/cross-env) from 7.0.2 to 7.0.3.
- [Release notes](https://github.com/kentcdodds/cross-env/releases)
- [Changelog](https://github.com/kentcdodds/cross-env/blob/master/CHANGELOG.md)
- [Commits](kentcdodds/cross-env@v7.0.2...v7.0.3)

---
updated-dependencies:
- dependency-name: cross-env
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps-dev): bump @babel/plugin-transform-modules-commonjs (#4562)

Bumps [@babel/plugin-transform-modules-commonjs](https://github.com/babel/babel/tree/HEAD/packages/babel-plugin-transform-modules-commonjs) from 7.23.3 to 7.24.1.
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.24.1/packages/babel-plugin-transform-modules-commonjs)

---
updated-dependencies:
- dependency-name: "@babel/plugin-transform-modules-commonjs"
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps-dev): bump jest-fail-on-console from 3.1.1 to 3.2.0 (#4563)

Bumps [jest-fail-on-console](https://github.com/ValentinH/jest-fail-on-console) from 3.1.1 to 3.2.0.
- [Release notes](https://github.com/ValentinH/jest-fail-on-console/releases)
- [Commits](ValentinH/jest-fail-on-console@v3.1.1...v3.2.0)

---
updated-dependencies:
- dependency-name: jest-fail-on-console
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* feat(FeatureFlags): broaden feature flag type to accept undefined (#4554)

* feat(FeatureFlags): loosen feature flag type to accept undefined

* chore: add changeset

* Update .changeset/grumpy-coats-worry.md

---------

Co-authored-by: Josh Black <joshblack@users.noreply.github.com>

* prevent form submit (#4551)

* deprecate title prop on ActionList.Group component on docs (#4544)

* chore: add hydro analytics to storybook (#4558)

* chore: add hydro analytics to storybook

* chore: use previewHead over managerHead

* Update build-docs

---------

Co-authored-by: Josh Black <joshblack@users.noreply.github.com>

* Revert "Revert "Add support for nested submenus to `ActionMenu`"" (#4486)

* Revert "Revert "Add support for nested submenus to `ActionMenu` (#4386)" (#4472)"

This reverts commit 82072eb.

* just want a change to trigger rebuild

---------

Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>
Co-authored-by: Pavithra Kodmad <pksjce@github.com>

* chore(deps): update typescript to 5.4.5 (#4568)

Co-authored-by: Josh Black <joshblack@users.noreply.github.com>

* Use dynamic height and width for dialogs (#4567)

* Use dynamic height and width for dialogs

* Update tall-forks-bathe.md

---------

Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>

* Make asterisk default, update story scenarios

* Update packages/react/src/FormControl/FormControl.docs.json

Co-authored-by: Owen Niblock <owenniblock@github.com>

* Update packages/react/src/internal/components/InputLabel.tsx

Co-authored-by: Owen Niblock <owenniblock@github.com>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Lukas Oppermann <lukasoppermann@github.com>
Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>
Co-authored-by: lukasoppermann <lukasoppermann@users.noreply.github.com>
Co-authored-by: Josh Black <joshblack@github.com>
Co-authored-by: Josh Black <joshblack@users.noreply.github.com>
Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com>
Co-authored-by: Armağan <broccolinisoup@github.com>
Co-authored-by: Ian Sanders <iansan5653@github.com>
Co-authored-by: Pavithra Kodmad <pksjce@github.com>
Co-authored-by: Dusty Greif <dgreif@users.noreply.github.com>
Co-authored-by: Owen Niblock <owenniblock@github.com>
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.

2 participants