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(manifest): add icons, tools & projectId #7980

Merged
merged 9 commits into from
Dec 16, 2024
Merged

Conversation

joshuaellis
Copy link
Member

@joshuaellis joshuaellis commented Dec 9, 2024

Description

Building on the work in #7403, the Content OS requires more information from the studio. Namely icons & tool information but ideally the projectId as well (although we could add this server side if this was preferable).

  • Un-deprecates Tool['icon'] but it is still considered optional.
  • Serialises tool information in separate file similar to how schemas are treated.
  • Serialises icons in a "generic" manor similar to visual editing, we expect this to cover most use-cases however we catch it if the serialisation fails and returns null as this part of the process should not cause overall failure.
  • Adds internal property to tools __internalApplicationType so we can group multiple across workspaces with the future ambitions these become applications. This is undocumented and will not impact plugin authors.

To avoid breaking change issues with create, even though the manifest is now used for more generic purposes we've kept the same naming conventions.

Related issues (and context)

  • resolves ENTX-2507
  • resolves ENTX-2572

What to review

  • Should we set the version to 1.1?
  • Is there an issue with including the projectId in the manifest files?
  • Is there concern over un-deprecating icons in tools?
  • Is there a foreseen issue adding type to tools?

Testing

Manual testing

In a project you can deploy using sanity deploy:

  • npm i sanity@content-os
  • npm run deploy (or npm run build && npm run start to run on localhost)
  • Visit /static/create-manifest.json
  • Visit a tools manifest (/static/ on the form .create-tools.json)

Known issues

Notes for release

Not required.

@joshuaellis joshuaellis requested a review from a team as a code owner December 9, 2024 09:54
Copy link

vercel bot commented Dec 9, 2024

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 Dec 16, 2024 4:50pm
performance-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 16, 2024 4:50pm
test-next-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 16, 2024 4:50pm
test-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 16, 2024 4:50pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
studio-workshop ⬜️ Ignored (Inspect) Visit Preview Dec 16, 2024 4:50pm

Copy link
Contributor

github-actions bot commented Dec 9, 2024

No changes to documentation

Copy link
Contributor

github-actions bot commented Dec 9, 2024

Component Testing Report Updated Dec 16, 2024 4:44 PM (UTC)

❌ Failed Tests (2) -- expand for details
File Status Duration Passed Skipped Failed
comments/CommentInput.spec.tsx ✅ Passed (Inspect) 1m 21s 15 0 0
formBuilder/ArrayInput.spec.tsx ✅ Passed (Inspect) 14s 3 0 0
formBuilder/inputs/PortableText/Annotations.spec.tsx ✅ Passed (Inspect) 40s 6 0 0
formBuilder/inputs/PortableText/copyPaste/CopyPaste.spec.tsx ✅ Passed (Inspect) 56s 11 7 0
formBuilder/inputs/PortableText/copyPaste/CopyPasteFields.spec.tsx ✅ Passed (Inspect) 0s 0 12 0
formBuilder/inputs/PortableText/Decorators.spec.tsx ✅ Passed (Inspect) 28s 6 0 0
formBuilder/inputs/PortableText/DisableFocusAndUnset.spec.tsx ✅ Passed (Inspect) 15s 3 0 0
formBuilder/inputs/PortableText/DragAndDrop.spec.tsx ❌ Failed (Inspect) 1m 48s 4 0 2
formBuilder/inputs/PortableText/FocusTracking.spec.tsx ✅ Passed (Inspect) 1m 14s 15 0 0
formBuilder/inputs/PortableText/Input.spec.tsx ✅ Passed (Inspect) 3m 3s 21 0 0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx ✅ Passed (Inspect) 1m 49s 18 0 0
formBuilder/inputs/PortableText/PresenceCursors.spec.tsx ✅ Passed (Inspect) 14s 3 9 0
formBuilder/inputs/PortableText/RangeDecoration.spec.tsx ✅ Passed (Inspect) 42s 9 0 0
formBuilder/inputs/PortableText/Styles.spec.tsx ✅ Passed (Inspect) 29s 6 0 0
formBuilder/inputs/PortableText/Toolbar.spec.tsx ✅ Passed (Inspect) 1m 53s 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

Copy link
Contributor

github-actions bot commented Dec 9, 2024

⚡️ Editor Performance Report

Updated Mon, 16 Dec 2024 16:44:40 GMT

Benchmark reference
latency of sanity@latest
experiment
latency of this branch
Δ (%)
latency difference
article (title) 25.0 efps (40ms) 23.3 efps (43ms) +3ms (+7.5%)
article (body) 75.2 efps (13ms) 73.0 efps (14ms) +0ms (-/-%)
article (string inside object) 24.4 efps (41ms) 27.0 efps (37ms) -4ms (-9.8%)
article (string inside array) 21.7 efps (46ms) 22.7 efps (44ms) -2ms (-4.3%)
recipe (name) 51.3 efps (20ms) 52.6 efps (19ms) -1ms (-2.6%)
recipe (description) 58.8 efps (17ms) 62.5 efps (16ms) -1ms (-5.9%)
recipe (instructions) 99.9+ efps (5ms) 99.9+ efps (5ms) +0ms (-/-%)
synthetic (title) 18.9 efps (53ms) 19.2 efps (52ms) -1ms (-1.9%)
synthetic (string inside object) 18.9 efps (53ms) 18.9 efps (53ms) +0ms (-/-%)

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) 40ms 43ms 62ms 361ms 536ms 9.9s
article (body) 13ms 15ms 17ms 68ms 50ms 4.8s
article (string inside object) 41ms 43ms 45ms 213ms 230ms 7.0s
article (string inside array) 46ms 49ms 52ms 165ms 152ms 7.2s
recipe (name) 20ms 21ms 23ms 32ms 0ms 6.9s
recipe (description) 17ms 18ms 20ms 22ms 0ms 4.4s
recipe (instructions) 5ms 7ms 7ms 8ms 0ms 3.1s
synthetic (title) 53ms 55ms 59ms 259ms 527ms 13.1s
synthetic (string inside object) 53ms 53ms 54ms 70ms 136ms 7.1s

🧪 Experiment result

The performance result of this branch

Benchmark latency p75 p90 p99 blocking time test duration
article (title) 43ms 45ms 47ms 251ms 284ms 12.0s
article (body) 14ms 15ms 16ms 25ms 63ms 4.5s
article (string inside object) 37ms 40ms 42ms 46ms 0ms 6.3s
article (string inside array) 44ms 45ms 49ms 213ms 237ms 7.0s
recipe (name) 19ms 21ms 22ms 35ms 0ms 6.9s
recipe (description) 16ms 17ms 18ms 21ms 0ms 4.3s
recipe (instructions) 5ms 6ms 7ms 9ms 0ms 3.0s
synthetic (title) 52ms 55ms 56ms 129ms 230ms 12.3s
synthetic (string inside object) 53ms 54ms 58ms 288ms 529ms 7.8s

📚 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.

@joshuaellis joshuaellis changed the title feat(manifest) add icons, tools & projectId feat(manifest): add icons, tools & projectId Dec 9, 2024
@joshuaellis joshuaellis force-pushed the feat/manifest-additions branch from bddcb59 to 0edb847 Compare December 9, 2024 10:50
@joshuaellis joshuaellis force-pushed the feat/manifest-additions branch from 0edb847 to aec2da0 Compare December 9, 2024 12:36
@mariuslundgard mariuslundgard requested a review from a team December 10, 2024 10:15
@joshuaellis
Copy link
Member Author

NOTE: if we approve this PR, I will make the necessary follow ups to tools that don't exist in this monorepo e.g. media.

Copy link
Contributor

@snorrees snorrees left a comment

Choose a reason for hiding this comment

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

Have not manually tested yet, will come back for that:

Should we set the version to 1.1?

Design flaw one: it is hardcode to version: 1.

Do we just break the format right away and make it version: string? Then you set '1.1' when writing the file? Or do we continue the design as is and hardcode version: 2 possibly version: 1.1, but I think the former fits better if we go with number.

Version is not used anywhere atm, so open to suggestions. Maybe just increment the number when we change things, ie 2, would be my gut feeling atm.

Is there an issue with including the projectId in the manifest files?

I dont see any problem with this. Its not a secret. Lets not discuss the org/project paradigm here (in public).

Is there concern over un-deprecating icons in tools?

I'll let Studio team answer this one. It could be seen as a bit odd to have a field in the config-api that is not used by Studio itself, so you could argue it should be nested under a new options property or something.

@joshuaellis joshuaellis force-pushed the feat/manifest-additions branch from 30952a9 to c1a548a Compare December 16, 2024 15:50
Copy link

socket-security bot commented Dec 16, 2024

New dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/[email protected] Transitive: eval, filesystem, network, shell, unsafe +18 8.7 MB kkomelin

View full report↗︎

Copy link
Member

@bjoerge bjoerge left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@joshuaellis joshuaellis added this pull request to the merge queue Dec 16, 2024
Merged via the queue into next with commit 8d5fb58 Dec 16, 2024
55 of 56 checks passed
@joshuaellis joshuaellis deleted the feat/manifest-additions branch December 16, 2024 16:49
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