-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
WIP: incremental payloads UI #3601
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 6f3fcb3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 |
return ( | ||
<div | ||
className="graphiql-increment-editor" | ||
style={isOpen ? { height: '30vh' } : {}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the editors to show properly I needed to give the wrapper a fixed height, which is not ideal. But I don't see a better way right now other than starting to do JS magic to calculate the container height on-runtime.
onClick={toggleEditor} | ||
> | ||
{props.isInitial ? 'Initial payload' : 'Increment'} (after{' '} | ||
{props.increment.timing / 1000}s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: Show in ms if the timing is <1s, otherwise show seconds
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3601 +/- ##
==========================================
- Coverage 55.32% 54.79% -0.53%
==========================================
Files 115 117 +2
Lines 5352 5409 +57
Branches 1451 1472 +21
==========================================
+ Hits 2961 2964 +3
- Misses 1965 2017 +52
- Partials 426 428 +2
|
0f5ab70
to
074061a
Compare
The latest changes of this PR are available as canary in npm (based on the declared
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
trivial, for humans
// but technically it can also be a query or mutation where the fetcher | ||
// decided to return an observable with either a single payload or | ||
// multiple payloads (defer/stream). As the naming is part of the | ||
// public API of this context we decide not to change it until defer/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// public API of this context we decide not to change it until defer/ | |
// public API of this context, we decided not to change it until defer/ |
How it looks like for
@defer
/@stream
: https://twitter.com/heyenbrock/status/1790134466960093476How it looks like for subscriptions:
CleanShot.2024-05-14.at.12.14.27.mp4
TODOs: