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

feat: add context / dropdown menu to component library #1557

Merged
merged 8 commits into from May 2, 2024

Conversation

hwkr
Copy link
Contributor

@hwkr hwkr commented Apr 30, 2024

Todo:

  • Make tests actually test something
  • Extract resize logic into a hook?
  • Create checkbox thing in component library? Or maybe it should be in the other repo?
Google Chrome-2024-04-29-19-30-12@2x

@hwkr hwkr requested review from amritk and geoffgscott April 30, 2024 02:30
@hwkr hwkr requested a review from marclave as a code owner April 30, 2024 02:30
Copy link

changeset-bot bot commented Apr 30, 2024

⚠️ No Changeset found

Latest commit: a47ac00

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

Copy link

relativeci bot commented Apr 30, 2024

#1264 Bundle Size — 1.9MiB (~+0.01%).

a47ac00(current) vs 7c443eb main#1260(baseline)

Warning

Bundle contains 4 duplicate packages – View duplicate packages

Bundle metrics  Change 2 changes Regression 1 regression
                 Current
#1264
     Baseline
#1260
Regression  Initial JS 1.9MiB(~+0.01%) 1.9MiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 100% 0%
No change  Chunks 1 1
No change  Assets 1 1
No change  Modules 1045 1045
No change  Duplicate Modules 0 0
No change  Duplicate Code 0% 0%
No change  Packages 158 158
No change  Duplicate Packages 4 4
Bundle size by type  Change 1 change Regression 1 regression
                 Current
#1264
     Baseline
#1260
Regression  JS 1.9MiB (~+0.01%) 1.9MiB

Bundle analysis reportBranch brynn/doc-1702-contextdropdown-m...Project dashboard

@hanspagel hanspagel marked this pull request as draft April 30, 2024 11:27
@hanspagel
Copy link
Member

I just make it a draft until we worked on the remaining tasks if that’s OK. ✌️

@hwkr hwkr force-pushed the brynn/doc-1702-contextdropdown-menu branch from 9f26448 to fceba12 Compare May 1, 2024 19:44
@hwkr hwkr marked this pull request as ready for review May 1, 2024 21:28
Copy link
Contributor

@geoffgscott geoffgscott left a comment

Choose a reason for hiding this comment

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

Looks great once we at the MaybeRefOrGetter stuff

}

export function useResizeWithTarget(
target: Ref<Element | undefined>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Best to use MayebRefOrGetter in hooks for for flexible composition
This then requires toValue(target) when using the value.

@hwkr hwkr force-pushed the brynn/doc-1702-contextdropdown-menu branch from e1b89d8 to 070e35a Compare May 2, 2024 19:33
Copy link
Contributor

github-actions bot commented May 2, 2024

@geoffgscott geoffgscott merged commit 3191eae into main May 2, 2024
9 checks passed
@geoffgscott geoffgscott deleted the brynn/doc-1702-contextdropdown-menu branch May 2, 2024 20:24
@hwkr hwkr mentioned this pull request May 2, 2024
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.

None yet

4 participants