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(form): add support for disabling array input capabilities #7615

Merged
merged 4 commits into from
Jan 20, 2025
Merged

Conversation

bjoerge
Copy link
Member

@bjoerge bjoerge commented Oct 14, 2024

Description

This adds support for the disableActions option on array fields for disabling various array input capabilities like:

  • add – Removes the ability to add new items to the array
  • addBefore – Removes the "Add item before"-menu item from the array item menu
  • addAfter – Removes the "Add item after"-menu item from the array item menu
  • remove – Removes the ability to remove items from the array
  • duplicate – Removes the ability to duplicate array items
  • copy – Removes the ability to copy items from the array

Usage example:

{
      name: 'someArrayField',
      options: {
        disableActions: ['add', 'duplicate'],
      },
      title: "Array you can't add elements to",
      type: 'array',
      of: [
        {
          type: 'object',
          name: 'something',
          title: 'Something',
          fields: [{name: 'first', type: 'string', title: 'First string'}],
        },
      ],
    }

Things to note:

  • These changes are only about UI affordances, and doesn't imply any form of write protection for the actual data. Items can still be added or removed to an array by sending mutations to the API.
  • This only applies to arrays of objects and arrays of primitive values. Not to portable text arrays.
  • Disabling add will also implicitly disable addBefore and addAfter, thus disable inserting new items to the array entirely (although items can still be inserted via duplicate).
  • There might not be a compelling use case for disabling the ability to copy here(?), but including it for completeness. If all known actions are disabled, the menu items and "add"-affordances will be removed from the UI.
  • If we later decide to introduce a new default action, this will appear after upgrading the Studio, and will have to be explicitly disabled.
  • Disabling copy will also disable the copy field action (for the entire array). In the same vein, disabling add will remove the "paste" field action. However, any other field actions that may have been added by plugins etc. may still be able to manipulate the array in ways disallowed by disableActions

What to review

Does it make sense?

Testing

  • Disable each of the capabilities and check that it works as expected. Toggle individual capabilities in dev/test-studio/schema/debug/arrayCapabilities.ts
    I've also added a basic e2e test that asserts that menu items and add button doesn't appear when all capabilities are disabled

Notes for release

  • Adds support for the disableActions option on array fields for disabling various array input capabilities

Copy link

vercel bot commented Oct 14, 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 Jan 17, 2025 10:25am
performance-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 17, 2025 10:25am
test-compiled-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 17, 2025 10:25am
test-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 17, 2025 10:25am
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
studio-workshop ⬜️ Ignored (Inspect) Visit Preview Jan 17, 2025 10:25am
test-next-studio ⬜️ Ignored (Inspect) Visit Preview Jan 17, 2025 10:25am

@bjoerge bjoerge requested review from mmgj and EoinFalconer October 14, 2024 14:53
Copy link
Contributor

No changes to documentation

Copy link
Contributor

github-actions bot commented Oct 14, 2024

Component Testing Report Updated Jan 17, 2025 10:28 AM (UTC)

❌ Failed Tests (2) -- expand for details
File Status Duration Passed Skipped Failed
comments/CommentInput.spec.tsx ✅ Passed (Inspect) 1m 7s 15 0 0
formBuilder/ArrayInput.spec.tsx ✅ Passed (Inspect) 12s 3 0 0
formBuilder/inputs/PortableText/Annotations.spec.tsx ❌ Failed (Inspect) 1m 59s 4 0 2
formBuilder/inputs/PortableText/copyPaste/CopyPaste.spec.tsx ✅ Passed (Inspect) 50s 11 7 0
formBuilder/inputs/PortableText/copyPaste/CopyPasteFields.spec.tsx ✅ Passed (Inspect) 0s 0 12 0
formBuilder/inputs/PortableText/Decorators.spec.tsx ✅ Passed (Inspect) 25s 6 0 0
formBuilder/inputs/PortableText/DisableFocusAndUnset.spec.tsx ✅ Passed (Inspect) 14s 3 0 0
formBuilder/inputs/PortableText/DragAndDrop.spec.tsx ✅ Passed (Inspect) 26s 6 0 0
formBuilder/inputs/PortableText/FocusTracking.spec.tsx ✅ Passed (Inspect) 1m 6s 15 0 0
formBuilder/inputs/PortableText/Input.spec.tsx ✅ Passed (Inspect) 1m 32s 21 0 0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx ✅ Passed (Inspect) 2m 1s 21 0 0
formBuilder/inputs/PortableText/PresenceCursors.spec.tsx ✅ Passed (Inspect) 13s 3 9 0
formBuilder/inputs/PortableText/Styles.spec.tsx ✅ Passed (Inspect) 26s 6 0 0
formBuilder/inputs/PortableText/Toolbar.spec.tsx ✅ Passed (Inspect) 1m 42s 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 Oct 14, 2024

⚡️ Editor Performance Report

Updated Fri, 17 Jan 2025 10:29:59 GMT

Benchmark reference
latency of sanity@latest
experiment
latency of this branch
Δ (%)
latency difference
article (title) 25.6 efps (39ms) 26.3 efps (38ms) -1ms (-2.6%)
article (body) 72.5 efps (14ms) 77.5 efps (13ms) -1ms (-/-%)
article (string inside object) 27.0 efps (37ms) 27.0 efps (37ms) +0ms (-/-%)
article (string inside array) 23.8 efps (42ms) 24.4 efps (41ms) -1ms (-2.4%)
synthetic (title) 18.2 efps (55ms) 19.2 efps (52ms) -3ms (-5.5%)
synthetic (string inside object) 18.9 efps (53ms) 19.2 efps (52ms) -1ms (-1.9%)

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) 39ms 41ms 55ms 428ms 855ms 10.9s
article (body) 14ms 16ms 22ms 161ms 218ms 5.3s
article (string inside object) 37ms 39ms 41ms 48ms 0ms 6.3s
article (string inside array) 42ms 44ms 49ms 259ms 261ms 7.0s
synthetic (title) 55ms 59ms 65ms 281ms 807ms 13.8s
synthetic (string inside object) 53ms 56ms 66ms 452ms 868ms 8.3s

🧪 Experiment result

The performance result of this branch

Benchmark latency p75 p90 p99 blocking time test duration
article (title) 38ms 41ms 68ms 448ms 710ms 10.5s
article (body) 13ms 16ms 17ms 56ms 56ms 4.6s
article (string inside object) 37ms 39ms 45ms 226ms 285ms 6.5s
article (string inside array) 41ms 43ms 45ms 147ms 157ms 6.8s
synthetic (title) 52ms 54ms 59ms 308ms 763ms 12.7s
synthetic (string inside object) 52ms 55ms 66ms 119ms 1229ms 8.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.

@bjoerge bjoerge changed the title feat(form): add support for disabling various array input capabilities feat(form): add support for disabling array input capabilities Jan 13, 2025
@bjoerge bjoerge marked this pull request as ready for review January 13, 2025 15:31
@bjoerge bjoerge requested a review from a team as a code owner January 13, 2025 15:31
@bjoerge bjoerge requested a review from jordanl17 January 13, 2025 15:32
pedrobonamin
pedrobonamin previously approved these changes Jan 15, 2025
Copy link
Contributor

@pedrobonamin pedrobonamin left a comment

Choose a reason for hiding this comment

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

Nice work, looks good to me!

Comment on lines 116 to 117
!disableActions.includes('add') &&
!(disableActions.includes('addAfter') && disableActions.includes('addBefore')) && (
Copy link
Member

Choose a reason for hiding this comment

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

Q: This seems different to the other conditions added elsewhere in this PR (eg here) where we only allow after if add and addAfter are not disabled.

Is it intended for this to be different?

Copy link
Member Author

Choose a reason for hiding this comment

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

Great catch! Fixed in 991d357

Copy link
Member

@jordanl17 jordanl17 left a comment

Choose a reason for hiding this comment

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

Nice one @bjoerge

@bjoerge bjoerge added this pull request to the merge queue Jan 20, 2025
Merged via the queue into next with commit 9ce399c Jan 20, 2025
55 of 56 checks passed
@bjoerge bjoerge deleted the sdx-1641 branch January 20, 2025 16:05
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