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

Suggestion: optional maxHeight property for Container to override default #194

Open
walmink opened this issue Feb 19, 2025 · 1 comment
Open

Comments

@walmink
Copy link

walmink commented Feb 19, 2025

The problem

Currently, the sheet container cannot get any taller than the MAX_HEIGHT defined in constants.ts:

export const MAX_HEIGHT = 'calc(100% - env(safe-area-inset-top) - 34px)';

That 34px feels a bit arbitrary :)

It means that, afaik, there is no way to let the sheet extend:

  • To the top of the screen (if you want a full-screen sheet)
  • Beyond the safe area at the top (if you are building a web app and on mobile you want a true full-screen sheet)

A solution

In the SheetContainer.tsx, add maxHeight as an optional parameter for the component:

export const SheetContainer = forwardRef<any, SheetContainerProps>(
    ({ children, style = {}, className = '', maxHeight = MAX_HEIGHT, ...rest }, ref) => {
      ...
      ...
      ...
    }
);

Then when determining the height, use the new maxHeight to determine the height:

const height =
  maxSnapHeight !== null
    ? `min(${maxSnapHeight}px, ${maxHeight})`
    : maxHeight;

Next steps

I tested this change and it works nicely. The user will need to provide the height as a string, otherwise it won't work. It could be improved by also supporting a number and adding px if the value is a number (or % if that feels more intuitive).

I've made a branch with this change, but cannot push. Happy to help if someone can give me the necessary permissions :)

@walmink
Copy link
Author

walmink commented Feb 19, 2025

P.S. While I get that sheets are typically capped near the top of the (safe) viewport, I think this a useful addition because occasionally you want a sheet to take over the screen and it's a shame you can't use this library for that, because it has amazing interaction handling. This addition keeps the default as before, so nothing more is expected from the dev unless they really want to change the maxHeight.

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

No branches or pull requests

1 participant