docs: add Sistent Permission component documentation - #7933
docs: add Sistent Permission component documentation#7933dhruveshmishra wants to merge 11 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds documentation for the Sistent PermissionShield (Permission) component, addressing #7927.
Changes:
- Added a Permission component landing page with examples for inline overlay and badge variants.
- Added usage/gating guidance (disable vs. hide) and best practices.
- Added integration/code samples and an API/props reference table.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 12 comments.
| File | Description |
|---|---|
| src/collections/sistent/components/permission/index.mdx | Adds overview + inline/badge variant examples for PermissionShield |
| src/collections/sistent/components/permission/guidance.mdx | Adds design guidance and best practices for permission gating |
| src/collections/sistent/components/permission/code.mdx | Adds installation, usage examples, and props/API reference |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds Sistent documentation for ChangesPermission component documentation
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
3eac6de to
b90c441
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/collections/sistent/components/permission/code.mdx (1)
57-60: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winFix the
PermissionShieldAPI props table.
PermissionShielddoes not exposePermissionKeySpec,boundaryPadding, ordefaultin@sistent/sistent0.21.44; its props arepermissionKey: Key,children, and optionalvariant?: 'inline' | 'badge'. Replace the table row with the actual API around lines 85-90.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/collections/sistent/components/permission/code.mdx` around lines 57 - 60, Update the PermissionShield API props table near the documented example to match `@sistent/sistent` 0.21.44: document permissionKey as Key, children, and optional variant with inline or badge values. Remove references to PermissionKeySpec, boundaryPadding, and default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/collections/sistent/components/permission/code.mdx`:
- Around line 23-45: Update the basic-permission-shield example and its
CodeBlock snippet to include the required SistentThemeProvider and a denied
permission state that visibly triggers the lock badge. Keep the
PermissionShield/Button usage intact and make the copied snippet self-contained,
or explicitly document both prerequisites within it.
In `@src/collections/sistent/components/permission/guidance.mdx`:
- Line 48: Update the guidance around “Interactive Closeness” to instruct
disabling nested controls directly, using preventDefault() where necessary
instead of relying on propagation stopping. For custom controls, also specify
keyboard handling and aria-disabled so pointer and keyboard interactions cannot
trigger actions.
In `@src/collections/sistent/components/permission/index.mdx`:
- Line 11: Add an explicit authorization disclaimer to the PermissionShield
documentation in the overview, guidance.mdx, and code.mdx: clarify that it is
presentation-layer UI gating only, client-side checks must not decide access,
and the API, routes, and server must enforce permissions on every request.
---
Nitpick comments:
In `@src/collections/sistent/components/permission/code.mdx`:
- Around line 57-60: Update the PermissionShield API props table near the
documented example to match `@sistent/sistent` 0.21.44: document permissionKey as
Key, children, and optional variant with inline or badge values. Remove
references to PermissionKeySpec, boundaryPadding, and default.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 06d65de1-f722-435b-92d2-50ce8f5f6e95
📒 Files selected for processing (3)
src/collections/sistent/components/permission/code.mdxsrc/collections/sistent/components/permission/guidance.mdxsrc/collections/sistent/components/permission/index.mdx
Signed-off-by: dhruveshmishra <dhruveshmishra09@gmail.com>
f8d73fa to
d9769ea
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
@rishiraj38 ^^ |
|
@dhruveshmishra Please address the AI suggestions provided. You can either confirm that you agree with them and consider them valid, or, if you disagree, clearly explain the reason for your disagreement. |
|
@Utkarsh-Mauryaa I had addressed all thr copilot suggestions already and u can see they are outdated mostly. Thanks |
Signed-off-by: dhruveshmishra <dhruveshmishra09@gmail.com>
8175cef to
e6bd037
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/collections/sistent/components/permission/code.mdx (1)
25-42:⚠️ Potential issue | 🟠 MajorMake the basic example conditional and copyable.
The previous review finding remains unresolved. Both examples render
PermissionShieldaround an enabledButtonwithout showing the consumer-side permission decision. In@sistent/sistentv0.21.46,PermissionShieldis visual-only. It does not check authorization or set the child’sdisabledstate. It applies reduced opacity and disabled pointer events to the child. (github.com)Render the shield only when the application denies access. Pass
disabledto theButton. AddPermissionProviderand the permission evaluator, or document those prerequisites. AddSistentThemeProviderto the copied snippet or state that the surrounding application must provide it. Sistent’s installation guidance requires this provider. (layer5.io)As per coding guidelines, MDX changes must meet WCAG 2.1 Level AA accessibility requirements.
Verification script
#!/usr/bin/env bash set -euo pipefail tmp="$(mktemp -d)" trap 'rm -rf "$tmp"' EXIT npm pack --silent `@sistent/sistent`@0.21.46 --pack-destination "$tmp" >/dev/null archive="$(find "$tmp" -maxdepth 1 -type f -name '*.tgz' -print -quit)" tar -xzf "$archive" -C "$tmp" rg -n -C 4 'PermissionShieldProps|PermissionProvider|SistentThemeProvider|pointerEvents' "$tmp/package"Also applies to: 45-61
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/collections/sistent/components/permission/code.mdx` around lines 25 - 42, Update both PermissionShield examples to demonstrate the consumer-side permission decision: evaluate access, pass the result to Button’s disabled prop, and render PermissionShield only when access is denied. Make each snippet copyable by including SistentThemeProvider and PermissionProvider with the evaluator, or explicitly document those required surrounding providers; preserve accessible disabled-button behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/collections/sistent/components/permission/code.mdx`:
- Around line 25-42: Update both PermissionShield examples to demonstrate the
consumer-side permission decision: evaluate access, pass the result to Button’s
disabled prop, and render PermissionShield only when access is denied. Make each
snippet copyable by including SistentThemeProvider and PermissionProvider with
the evaluator, or explicitly document those required surrounding providers;
preserve accessible disabled-button behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ceee9ebf-856f-4da2-a89c-1f47f2e1a133
📒 Files selected for processing (2)
src/collections/sistent/components/permission/code.mdxsrc/collections/sistent/components/permission/index.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/collections/sistent/components/permission/index.mdx
| <h2>Custom Boundary Padding</h2> | ||
| </a> | ||
|
|
||
| Use the `boundaryPadding` prop to configure custom offsets if the tooltip overlaps fixed elements (like headers, sidebars, or navbar sections) during scrolling. |
There was a problem hiding this comment.
I think boundaryPadding does not exist in the codebase. Can you check again in the layer5io/sistent repo?
There was a problem hiding this comment.
The boundaryPadding prop is currently on the fixing-tools branch of the sistent repository introduced in the tooltip responsiveness fixes and will be available once that branch is merged into master
rishiraj38
left a comment
rishiraj38
left a comment
There was a problem hiding this comment.
@dhruveshmishra, thanks for working on this! Could you please check Sistent once again? There have been a few changes made to this component, so please update your PR accordingly.
|
sure @rishiraj38 |
|
Preview deployment for PR #7933 removed. This PR preview was automatically pruned because we keep only the 3 most recently updated previews on GitHub Pages to stay within deployment size limits. If needed, push a new commit to this PR to generate a fresh preview. |
…cent component changes Signed-off-by: dhruveshmishra <dhruveshmishra09@gmail.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/collections/sistent/components/permission/index.mdx (2)
24-36: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDisable the wrapped
Button.The guidance in
src/collections/sistent/components/permission/guidance.mdxrequires nested controls to handle disabled interaction independently. This example leaves theButtonenabled at Line [33]. Adddisabledbefore presenting this as a permission-restricted action.Proposed fix
- <Button variant="contained" color="error"> + <Button variant="contained" color="error" disabled>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/collections/sistent/components/permission/index.mdx` around lines 24 - 36, Update the Button nested within PermissionShield to include the disabled prop, ensuring the permission-restricted Delete Resource action is presented as non-interactive.
25-35: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign the button label with
permissionKey.The permission key describes deleting a connection, but the button says
Delete Resourceat Line [34]. UseDelete Connectionor update the permission metadata so the visible action and permission explanation describe the same operation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/collections/sistent/components/permission/index.mdx` around lines 25 - 35, Update the button text within the permissionKey block so it matches the described operation, using “Delete Connection” to align with the existing delete_connection metadata and description.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/collections/sistent/components/permission/index.mdx`:
- Around line 24-36: Update the Button nested within PermissionShield to include
the disabled prop, ensuring the permission-restricted Delete Resource action is
presented as non-interactive.
- Around line 25-35: Update the button text within the permissionKey block so it
matches the described operation, using “Delete Connection” to align with the
existing delete_connection metadata and description.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 95ab849f-8607-4346-bde4-155ddd7e5ba7
📒 Files selected for processing (3)
src/collections/sistent/components/permission/code.mdxsrc/collections/sistent/components/permission/guidance.mdxsrc/collections/sistent/components/permission/index.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/collections/sistent/components/permission/guidance.mdx
- src/collections/sistent/components/permission/code.mdx
There was a problem hiding this comment.
@dhruveshmishra, please update the docs. This is no longer how we're implementing the Permission Shield.
Please check the recent changes made to the Permission component. It now supports both tooltip and card variants, so it can be used as either a tooltip or a card. Please update the documentation accordingly.
…ission-shield-docs
…issionSessionContext Signed-off-by: dhruveshmishra <dhruveshmishra09@gmail.com>
|
@rishiraj38 I had made some changes kindly take a look on that |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/collections/sistent/components/permission/code.mdx`:
- Around line 134-148: Update the ForbiddenPage component’s “Access Denied”
heading from h2 to h1, preserving the existing text and surrounding layout.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 142b0baa-b419-4c79-9d9d-7cf61242ff4e
📒 Files selected for processing (3)
src/collections/sistent/components/permission/code.mdxsrc/collections/sistent/components/permission/guidance.mdxsrc/collections/sistent/components/permission/index.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/collections/sistent/components/permission/guidance.mdx
Signed-off-by: dhruveshmishra <dhruveshmishra09@gmail.com>
Description
This PR fixes #7927
Summary by CodeRabbit