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

Update overlay props #4261

Merged
merged 13 commits into from
Nov 1, 2024
Merged

Update overlay props #4261

merged 13 commits into from
Nov 1, 2024

Conversation

tgberkeley
Copy link
Collaborator

All Submissions:

  • Have you followed the guidelines stated in CONTRIBUTING.md file?
  • Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

New Feature Submission:

  • Does your submission pass the tests?
  • Have you linted your code locally prior to submission?

Changes To Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

After these steps, you're ready to open a pull request.

a. Give a descriptive title to your PR.

b. Describe your changes.

c. Put `closes #XXXX` in your comment to auto-close the issue that your PR fixes (if such).

# Whether the drawer is open or not.
open: Var[bool]

# Enable background scaling, it requires an element with [vaul-drawer-wrapper] data attribute to scale its background.
should_scale_background: Var[bool]
# Fires when the drawer is opened.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# Fires when the drawer is opened.
# Fires when the drawer is opened or closed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

# When `False`, it allows to interact with elements outside of the drawer without closing it. Defaults to `True`.
modal: Var[bool]

# Direction of the drawer. This adjust the animations and the drag direction. Defaults to `"bottom"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# Direction of the drawer. This adjust the animations and the drag direction. Defaults to `"bottom"`
# Direction of the drawer. This adjusts the animations and the drag direction. Defaults to `"bottom"`

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed


# Number between 0 and 1 that determines when the drawer should be closed.
close_threshold: Var[float]
# When `False`, it allows to interact with elements outside of the drawer without closing it. Defaults to `True`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# When `False`, it allows to interact with elements outside of the drawer without closing it. Defaults to `True`.
# When `False`, it allows interaction with elements outside of the drawer without closing it. Defaults to `True`.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

# Gets triggered after the open or close animation ends, it receives an open argument with the open state of the drawer by the time the function was triggered.
on_animation_end: EventHandler[identity_event(bool)]

# When false dragging, clicking outside, pressing esc, etc. will not close the drawer. Use this in combination with the open prop, otherwise you won't be able to open/close the drawer.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# When false dragging, clicking outside, pressing esc, etc. will not close the drawer. Use this in combination with the open prop, otherwise you won't be able to open/close the drawer.
# When `False`, dragging, clicking outside, pressing esc, etc. will not close the drawer. Use this in combination with the open prop, otherwise you won't be able to open/close the drawer.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

# When false dragging, clicking outside, pressing esc, etc. will not close the drawer. Use this in combination with the open prop, otherwise you won't be able to open/close the drawer.
dismissible: Var[bool]

# When true dragging will only be possible by the handle.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# When true dragging will only be possible by the handle.
# When `True`, dragging will only be possible by the handle.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

# When `True`, it prevents scroll restoration. Defaults to `True`.
preventScrollRestoration: Var[bool]

# Fires when the drawer is opened.
on_open_change: EventHandler[identity_event(bool)]
# Enable background scaling, it requires an element with [vaul-drawer-wrapper] data attribute to scale its background.
Copy link
Collaborator

Choose a reason for hiding this comment

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

unclear what this means, but based on https://www.shadcn-vue.com/docs/components/drawer#scale-background, it seems vaul-drawer-wrapper isn't a "data attribute", but just a regular attribute, likely applied via special_props=[Var("vaulDrawerWrapper")], but i haven't tested it

Suggested change
# Enable background scaling, it requires an element with [vaul-drawer-wrapper] data attribute to scale its background.
# Enable background scaling, it requires container element with `vaul-drawer-wrapper` attribute to scale its background.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

# Whether to hide the content when the trigger becomes fully occluded. Defaults to False.
hide_when_detached: Var[bool]

# Fired when the dialog is closed.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This description for on_close_auto_focus doesn't seem quite right

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

you're right, fixed

on_close_auto_focus: EventHandler[empty_event]

# Fired when the escape key is pressed.
on_escape_key_down: EventHandler[empty_event]

# Fired when a pointer down event happens outside the context menu.
# Fired when the pointer is down outside the dialog.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here and below the object is changed to "dialog", but this really is a "context menu"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed all 3

# When true, prevents the user from interacting with the item.
disabled: Var[bool]

# Optional text used for typeahead purposes. By default the typeahead behavior will use the .textContent of the item. Use this when the content is complex, or you have non-textual content inside.
Copy link
Collaborator

Choose a reason for hiding this comment

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

comment mentions .textContent, but the prop is called text_value, which seems confusing

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@tgberkeley tgberkeley merged commit b70f33d into main Nov 1, 2024
27 of 29 checks passed
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.

3 participants