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

[core] feat: OverlaysProvider #6674

Merged
merged 14 commits into from
Jan 25, 2024
Merged

[core] feat: OverlaysProvider #6674

merged 14 commits into from
Jan 25, 2024

Conversation

adidahiya
Copy link
Contributor

@adidahiya adidahiya commented Jan 24, 2024

Follow-up to #6656

Checklist

  • Includes tests
  • Update documentation
  • Backwards-compatibility for <Dialog>, <Drawer>, <Popover>, <Omnibar>

Changes proposed in this pull request:

  • Introduce <OverlaysProvider> component and useOverlayStack() hook to manage global overlay stack state
  • Use react-uid library to generate ids for Overlay2 instances
  • Update Overlay2 test suite to be compatible with new context/provider pattern
  • Add backwards-compatibility for overlays used outside of an <OverlayProvider> by utilizing useSyncExternalStore() (requires shimming in React 16) to reference overlay state in a global variable
    • see tests for useOverlayStack() with and without the suggested provider context

Reviewers should focus on:

No regressions in overlay-based components

@adidahiya
Copy link
Contributor Author

WIP continue fixing Overlay2 tests

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

packages/core/src/context/overlays/overlays-provider.md Outdated Show resolved Hide resolved
renderPageActions={this.renderPageActions}
renderViewSourceLinkText={this.renderViewSourceLinkText}
/>
<OverlaysProvider>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO in a future PR: expose a <BlueprintProvider> which does all 3

@adidahiya
Copy link
Contributor Author

fix Overlay2 tests

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya
Copy link
Contributor Author

Fix core:iso test:

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya adidahiya marked this pull request as ready for review January 25, 2024 16:48
@adidahiya adidahiya changed the title [core] feat: OverlayProvider [core] feat: OverlaysProvider Jan 25, 2024
@adidahiya adidahiya mentioned this pull request Jan 25, 2024
5 tasks
@adidahiya
Copy link
Contributor Author

Overlay2 backcompat

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya
Copy link
Contributor Author

address self-review, attempt to fix test

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya
Copy link
Contributor Author

Fix tests

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya
Copy link
Contributor Author

need to add the OverlaysProvider and useOverlayStack docs to the navigation sidebar:
image

@@ -73,6 +75,7 @@
"@blueprintjs/node-build-scripts": "workspace:^",
"@blueprintjs/test-commons": "workspace:^",
"@testing-library/react": "^12.1.5",
"@types/use-sync-external-store": "0.0.6",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note that this does not need to be in "dependencies" because it's not exposed in the generated types:

// useLegacyOverlayStack.d.ts
import React from "react";
import { type OverlayStackAction } from "../../context/overlays/overlaysProvider";
import type { UseOverlayStackReturnValue } from "./useOverlayStack";
/**
 * Public only for testing.
 */
export declare const dispatch: React.Dispatch<OverlayStackAction>;
/**
 * Legacy implementation of a global overlay stack which maintains state in a global variable.
 * This is used for backwards-compatibility with overlay-based components in Blueprint v5.
 * It will be removed in Blueprint v6 once `<OverlaysProvider>` is required.
 *
 * @see https://github.com/palantir/blueprint/wiki/Overlay2-migration
 */
export declare function useLegacyOverlayStack(): UseOverlayStackReturnValue;

@adidahiya
Copy link
Contributor Author

revert stopPropagationEvents removal

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya adidahiya merged commit 315061a into develop Jan 25, 2024
11 of 12 checks passed
@adidahiya adidahiya deleted the ad/overlay2-provider branch January 25, 2024 20:43
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

1 participant