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(sanity): add CapabilityGate component #8621

Merged

Conversation

juice49
Copy link
Contributor

@juice49 juice49 commented Feb 12, 2025

Description

This branch adds a CapabilityGate component that can be used to conditionally render other components based on the current rendering context. For example, to prevent the global user menu from rendering when the rendering context provides this capability. You can see this exact scenario in #8591.

What to review

Any issues with the component implementation?

Testing

There are added unit tests, and you can see the changes in action in #8591.

Copy link

vercel bot commented Feb 12, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
page-building-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 6, 2025 5:03pm
performance-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 6, 2025 5:03pm
test-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 6, 2025 5:03pm
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
studio-workshop ⬜️ Ignored (Inspect) Visit Preview Mar 6, 2025 5:03pm
test-next-studio ⬜️ Ignored (Inspect) Mar 6, 2025 5:03pm

Copy link
Contributor Author

juice49 commented Feb 12, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link
Contributor

No changes to documentation

Copy link
Contributor

github-actions bot commented Feb 12, 2025

⚡️ Editor Performance Report

Updated Thu, 06 Mar 2025 17:09:53 GMT

Benchmark reference
latency of sanity@latest
experiment
latency of this branch
Δ (%)
latency difference
article (title) 21.1 efps (48ms) 17.9 efps (56ms) +9ms (+17.9%)
article (body) 63.3 efps (16ms) 63.7 efps (16ms) -0ms (-/-%)
article (string inside object) 24.4 efps (41ms) 22.2 efps (45ms) +4ms (+9.8%)
article (string inside array) 20.8 efps (48ms) 20.8 efps (48ms) +0ms (-/-%)
recipe (name) 37.0 efps (27ms) 50.0 efps (20ms) -7ms (-25.9%)
recipe (description) 41.7 efps (24ms) 55.6 efps (18ms) -6ms (-25.0%)
recipe (instructions) 99.9+ efps (5ms) 99.9+ efps (5ms) +0ms (-/-%)
synthetic (title) 20.0 efps (50ms) 16.4 efps (61ms) +11ms (+22.0%) 🔴
synthetic (string inside object) 19.6 efps (51ms) 16.1 efps (62ms) +11ms (+21.6%) 🔴

efps — editor "frames per second". The number of updates assumed to be possible within a second.

Derived from input latency. efps = 1000 / input_latency

Detailed information

🏠 Reference result

The performance result of sanity@latest

Benchmark latency p75 p90 p99 blocking time test duration
article (title) 48ms 54ms 86ms 280ms 743ms 10.9s
article (body) 16ms 18ms 26ms 60ms 311ms 5.8s
article (string inside object) 41ms 43ms 45ms 55ms 35ms 6.7s
article (string inside array) 48ms 51ms 56ms 352ms 386ms 8.1s
recipe (name) 27ms 28ms 31ms 60ms 9ms 7.8s
recipe (description) 24ms 25ms 31ms 51ms 1ms 5.4s
recipe (instructions) 5ms 7ms 7ms 10ms 0ms 3.1s
synthetic (title) 50ms 51ms 56ms 155ms 475ms 12.3s
synthetic (string inside object) 51ms 54ms 59ms 298ms 709ms 7.8s

🧪 Experiment result

The performance result of this branch

Benchmark latency p75 p90 p99 blocking time test duration
article (title) 56ms 76ms 89ms 258ms 1407ms 12.3s
article (body) 16ms 17ms 17ms 33ms 60ms 5.3s
article (string inside object) 45ms 48ms 52ms 146ms 329ms 7.8s
article (string inside array) 48ms 51ms 58ms 78ms 427ms 7.6s
recipe (name) 20ms 22ms 26ms 42ms 6ms 7.1s
recipe (description) 18ms 19ms 20ms 34ms 0ms 5.0s
recipe (instructions) 5ms 7ms 8ms 10ms 0ms 3.2s
synthetic (title) 61ms 63ms 66ms 212ms 1655ms 14.0s
synthetic (string inside object) 62ms 65ms 76ms 111ms 1961ms 9.5s

📚 Glossary

column definitions

  • benchmark — the name of the test, e.g. "article", followed by the label of the field being measured, e.g. "(title)".
  • latency — the time between when a key was pressed and when it was rendered. derived from a set of samples. the median (p50) is shown to show the most common latency.
  • p75 — the 75th percentile of the input latency in the test run. 75% of the sampled inputs in this benchmark were processed faster than this value. this provides insight into the upper range of typical performance.
  • p90 — the 90th percentile of the input latency in the test run. 90% of the sampled inputs were faster than this. this metric helps identify slower interactions that occurred less frequently during the benchmark.
  • p99 — the 99th percentile of the input latency in the test run. only 1% of sampled inputs were slower than this. this represents the worst-case scenarios encountered during the benchmark, useful for identifying potential performance outliers.
  • blocking time — the total time during which the main thread was blocked, preventing user input and UI updates. this metric helps identify performance bottlenecks that may cause the interface to feel unresponsive.
  • test duration — how long the test run took to complete.

Copy link
Contributor

github-actions bot commented Feb 12, 2025

Component Testing Report Updated Mar 6, 2025 5:15 PM (UTC)

❌ Failed Tests (2) -- expand for details
File Status Duration Passed Skipped Failed
comments/CommentInput.spec.tsx ✅ Passed (Inspect) 1m 4s 15 0 0
formBuilder/ArrayInput.spec.tsx ✅ Passed (Inspect) 12s 3 0 0
formBuilder/inputs/PortableText/Annotations.spec.tsx ❌ Failed (Inspect) 1m 56s 4 0 2
formBuilder/inputs/PortableText/copyPaste/CopyPaste.spec.tsx ✅ Passed (Inspect) 41s 11 7 0
formBuilder/inputs/PortableText/copyPaste/CopyPasteFields.spec.tsx ✅ Passed (Inspect) 0s 0 12 0
formBuilder/inputs/PortableText/Decorators.spec.tsx ✅ Passed (Inspect) 24s 6 0 0
formBuilder/inputs/PortableText/DisableFocusAndUnset.spec.tsx ✅ Passed (Inspect) 13s 3 0 0
formBuilder/inputs/PortableText/DragAndDrop.spec.tsx ✅ Passed (Inspect) 26s 6 0 0
formBuilder/inputs/PortableText/FocusTracking.spec.tsx ✅ Passed (Inspect) 1m 5s 15 0 0
formBuilder/inputs/PortableText/Input.spec.tsx ✅ Passed (Inspect) 1m 34s 21 0 0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx ✅ Passed (Inspect) 1m 44s 21 0 0
formBuilder/inputs/PortableText/PresenceCursors.spec.tsx ✅ Passed (Inspect) 12s 3 9 0
formBuilder/inputs/PortableText/Styles.spec.tsx ✅ Passed (Inspect) 26s 6 0 0
formBuilder/inputs/PortableText/Toolbar.spec.tsx ✅ Passed (Inspect) 1m 37s 21 0 0
formBuilder/tree-editing/TreeEditing.spec.tsx ✅ Passed (Inspect) 0s 0 3 0
formBuilder/tree-editing/TreeEditingNestedObjects.spec.tsx ✅ Passed (Inspect) 0s 0 3 0

@juice49 juice49 force-pushed the feat/sapp-2363/studio-rendering-content-capability-gate branch from e23ed66 to f6ea950 Compare March 3, 2025 11:35
@juice49 juice49 force-pushed the feat/sapp-2363/studio-rendering-context branch from 6c79dc5 to 455d288 Compare March 3, 2025 11:35
@juice49 juice49 force-pushed the feat/sapp-2363/studio-rendering-context branch 6 times, most recently from 0774445 to 5785e3e Compare March 4, 2025 22:29
@juice49 juice49 force-pushed the feat/sapp-2363/studio-rendering-content-capability-gate branch from f6ea950 to f4321f9 Compare March 4, 2025 22:30
@juice49 juice49 force-pushed the feat/sapp-2363/studio-rendering-context branch from 5785e3e to 2798b2d Compare March 4, 2025 22:30
@juice49 juice49 force-pushed the feat/sapp-2363/studio-rendering-context branch from 54002b7 to 19b1d80 Compare March 5, 2025 10:04
@juice49 juice49 force-pushed the feat/sapp-2363/studio-rendering-content-capability-gate branch from f4321f9 to d1fef34 Compare March 5, 2025 10:08
Copy link
Contributor

github-actions bot commented Mar 5, 2025

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 42.03% 54349 / 129288
🔵 Statements 42.03% 54349 / 129288
🔵 Functions 46.93% 2729 / 5815
🔵 Branches 79.42% 10185 / 12824
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/sanity/src/core/components/CapabilityGate.tsx 100% 66.66% 100% 100%
Generated in workflow #31818 for commit 0e446c7 by the Vitest Coverage Report Action

{wrapper},
)

expect(screen.getByTestId('user-menu')).toBeTruthy()
Copy link
Member

Choose a reason for hiding this comment

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

NIT: .toBeInTheDocument() rather than .toBeTruthy()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Curious: is that just a style preference?

Copy link
Member

Choose a reason for hiding this comment

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

Functionally, in this case there's no difference - but if you were using queryByTestId instead then that throws instead of returns null - so mostly around consistency of approach I suppose. toBeTruthy/Falsy I feel are best avoided in many cases for the additional false positives they might capture eg "" 0 - again not relevant in this case where you are returning the element

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for explaining. Makes sense! 🙂 I'm actually usually very opposed to truthy/falsey checks, for the reasons you stated.

jordanl17
jordanl17 previously approved these changes Mar 6, 2025
@juice49 juice49 changed the base branch from feat/sapp-2363/studio-rendering-context to graphite-base/8621 March 6, 2025 15:50
@juice49 juice49 force-pushed the graphite-base/8621 branch from daf24a6 to b0c005a Compare March 6, 2025 15:50
@juice49 juice49 force-pushed the feat/sapp-2363/studio-rendering-content-capability-gate branch from b2df74b to 34943ef Compare March 6, 2025 15:50
@juice49 juice49 changed the base branch from graphite-base/8621 to next March 6, 2025 15:51
@juice49 juice49 dismissed jordanl17’s stale review March 6, 2025 15:51

The base branch was changed.

@juice49 juice49 force-pushed the feat/sapp-2363/studio-rendering-content-capability-gate branch from 34943ef to 0a6f7ba Compare March 6, 2025 15:51
@juice49 juice49 requested a review from jordanl17 March 6, 2025 16:28
@juice49 juice49 force-pushed the feat/sapp-2363/studio-rendering-content-capability-gate branch from 0a6f7ba to 0e446c7 Compare March 6, 2025 16:56
Copy link
Contributor Author

juice49 commented Mar 6, 2025

Merge activity

  • Mar 6, 11:56 AM EST: Graphite rebased this pull request as part of a merge.
  • Mar 6, 12:09 PM EST: Graphite couldn't merge this PR because it was not satisfying all requirements (Failed CI: 'playwright-ct-test (firefox, 1, 2)').
  • Mar 6, 12:20 PM EST: A user started a stack merge that includes this pull request via Graphite.
  • Mar 6, 12:21 PM EST: A user merged this pull request with Graphite.

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.

2 participants