Repository navigation
feat(ui): make OAuth consent scopes selectable #10134
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| '@clerk/ui': minor | ||
| --- | ||
|
|
||
| Allow users to select or clear optional scopes in `<OAuthConsent />`. Only selected scopes are submitted. When shown, `offline_access` remains checked and cannot be cleared. | ||
|
|
||
| Customize the scope checkboxes with the new `listGroupItemCheckbox` appearance element. Checkboxes align with the first line of descriptions that wrap onto multiple lines. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@clerk/ui': patch | ||
| --- | ||
|
|
||
| Fix a stray scrollbar on the `<OAuthConsent />` and `<OAuthDeviceVerification />` scope list when only one scope is shown. |
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -1,6 +1,7 @@ | ||||
| import type { ComponentProps } from 'react'; | ||||
|
|
||||
| import { Box, descriptors, Text } from '@/ui/customizables'; | ||||
| import { Box, CheckboxInput, descriptors, Flex, Text } from '@/ui/customizables'; | ||||
| import type { InternalTheme } from '@/ui/styledSystem'; | ||||
| import { common } from '@/ui/styledSystem'; | ||||
| import { colors } from '@/ui/utils/colors'; | ||||
|
|
||||
|
|
@@ -74,7 +75,7 @@ export function ListGroupContent({ | |||
| backgroundColor: t.colors.$colorBackground, | ||||
| overflowY: 'auto', | ||||
| ...common.unstyledScrollbar(t), | ||||
| '--fade-distance': '2.5rem', | ||||
| '--fade-distance': '2rem', | ||||
| '&::before, &::after': { | ||||
| content: '""', | ||||
| display: 'block', | ||||
|
|
@@ -131,6 +132,14 @@ export function ListGroupContent({ | |||
| ); | ||||
| } | ||||
|
|
||||
| const listGroupItemStyles = (t: InternalTheme) => ({ | ||||
| paddingInline: t.space.$3, | ||||
| paddingBlock: t.space.$2, | ||||
| borderTopWidth: t.borderWidths.$normal, | ||||
| borderTopStyle: t.borderStyles.$solid, | ||||
| borderTopColor: t.colors.$borderAlpha100, | ||||
| }); | ||||
|
|
||||
| export function ListGroupItem({ | ||||
| children, | ||||
| sx, | ||||
|
|
@@ -142,13 +151,9 @@ export function ListGroupItem({ | |||
| as='li' | ||||
| sx={[ | ||||
| t => ({ | ||||
| ...listGroupItemStyles(t), | ||||
| display: 'flex', | ||||
| alignItems: 'baseline', | ||||
| paddingInline: t.space.$3, | ||||
| paddingBlock: t.space.$2, | ||||
| borderTopWidth: t.borderWidths.$normal, | ||||
| borderTopStyle: t.borderStyles.$solid, | ||||
| borderTopColor: t.colors.$borderAlpha100, | ||||
| '&::before': { | ||||
| content: '""', | ||||
| display: 'inline-block', | ||||
|
|
@@ -179,3 +184,37 @@ export function ListGroupItemLabel(props: Omit<ComponentProps<typeof Text>, 'ele | |||
| /> | ||||
| ); | ||||
| } | ||||
|
|
||||
| export function ListGroupCheckboxItem({ | ||||
| children, | ||||
| ...props | ||||
| }: Pick<ComponentProps<typeof CheckboxInput>, 'checked' | 'value' | 'isDisabled' | 'onChange'> & { | ||||
| children: ComponentProps<typeof Text>['children']; | ||||
| }) { | ||||
| return ( | ||||
| <Box | ||||
| as='li' | ||||
| sx={listGroupItemStyles} | ||||
| elementDescriptor={descriptors.listGroupItem} | ||||
| > | ||||
| <Flex | ||||
| as='label' | ||||
| align='start' | ||||
| gap={2} | ||||
| sx={{ cursor: 'pointer', userSelect: 'none' }} | ||||
| > | ||||
| <CheckboxInput | ||||
| {...props} | ||||
| focusRing={false} | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
# Inspect the checkbox implementation and any shared focus styles; do not run repository code.
fd -i 'checkbox|input' packages/ui/src/customizables
rg -n -C 5 'focusRing|focus-visible|outline' packages/ui/src/customizables packages/ui/src/components/OAuthConsent/ListGroup.tsxRepository: clerk/javascript Length of output: 1193 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- imports and component context ---'
sed -n '1,235p' packages/ui/src/components/OAuthConsent/ListGroup.tsx
printf '%s\n' '--- CheckboxInput bindings ---'
rg -n -F --glob '*.{ts,tsx}' -- 'CheckboxInput' packages/ui packages/types packages | head -80
printf '%s\n' '--- focusRing declarations and styles ---'
rg -n -C 8 --glob '*.{ts,tsx}' -- 'focusRing' packages/ui packages/types | head -240Repository: clerk/javascript Length of output: 25325 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- Input primitive ---'
sed -n '1,220p' packages/ui/src/primitives/Input.tsx
printf '%s\n' '--- checkbox and focus styles ---'
sed -n '1,85p' packages/ui/src/baseTheme.ts
rg -n -C 12 --glob '*.{ts,tsx}' -- 'focusRing\s*:' packages/ui/src/styledSystem packages/ui/src | head -180Repository: clerk/javascript Length of output: 22280 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- checkbox shadow helper usage ---'
rg -n -C 10 --glob '*.{ts,tsx}' -- 'checkboxShadowStyles' packages/ui/src
printf '%s\n' '--- border variant definitions and checkbox handling ---'
rg -n -C 14 --glob '*.{ts,tsx}' -- 'borderVariants' packages/ui/src/styledSystem packages/ui/src | head -320
printf '%s\n' '--- customizable checkbox binding ---'
sed -n '1,70p' packages/ui/src/customizables/index.tsRepository: clerk/javascript Length of output: 28776 🏁 Script executed: #!/bin/bash
set -e
sed -n '81,145p' packages/ui/src/styledSystem/common.tsRepository: clerk/javascript Length of output: 2160 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- baseTheme references and theme element application ---'
rg -n -C 12 --glob '*.{ts,tsx}' -- 'baseTheme|checkboxShadowStyles|theme\.(elements|components)|checkbox:' packages/ui/src | head -320
printf '%s\n' '--- customizable/style application ---'
rg -n -C 12 --glob '*.{ts,tsx}' -- 'makeCustomizable|elementDescriptor|appearance' packages/ui/src/customizables packages/ui/src/styledSystem | head -360Repository: clerk/javascript Length of output: 41424 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- simpleStyles uses ---'
rg -n -C 8 --glob '*.{ts,tsx}' -- 'simpleStyles' packages/ui/src
printf '%s\n' '--- simple theme and OAuth consent appearance ---'
rg -n -C 12 --glob '*.{ts,tsx}' -- 'simpleTheme|OAuthConsent|oauthConsent' packages/ui/src | head -320
printf '%s\n' '--- checkbox focus styles in all appearance definitions ---'
rg -n -C 8 --glob '*.{ts,tsx}' -- 'checkbox|focus-visible' packages/ui/src/baseTheme.ts packages/ui/src/customizables | head -260Repository: clerk/javascript Length of output: 41253 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- neobrutalism theme ---'
sed -n '1,240p' packages/ui/src/themes/neobrutalism.ts
printf '%s\n' '--- theme and OAuth consent appearance wiring ---'
rg -n -C 10 --glob '*.{ts,tsx}' -- 'neobrutalism|Theme|appearanceKey.*oauthConsent|appearanceKey.*OAuth|__internal_oauthConsent' packages/ui/src/themes packages/ui/src/components/OAuthConsent packages/ui/src/contexts packages/ui/src/internal | head -360Repository: clerk/javascript Length of output: 28218 Preserve a visible keyboard focus indicator for simple-style themes. With a simple-style theme such as Remove 🐛 Suggested fix--- "a/packages/ui/src/components/OAuthConsent/ListGroup.tsx"
+++ "b/packages/ui/src/components/OAuthConsent/ListGroup.tsx"
@@ -203,10 +203,9 @@
gap={2}
sx={{ cursor: 'pointer', userSelect: 'none' }}
>
<CheckboxInput
{...props}
- focusRing={false}
elementDescriptor={descriptors.listGroupItemCheckbox}
sx={t => ({
flexShrink: 0,
cursor: 'pointer',📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||
| elementDescriptor={descriptors.listGroupItemCheckbox} | ||||
| sx={t => ({ | ||||
| flexShrink: 0, | ||||
| cursor: 'pointer', | ||||
| marginBlockStart: `calc((${t.fontSizes.$md} * ${t.lineHeights.$small} - ${t.sizes.$3x5}) / 2)`, | ||||
| })} | ||||
| /> | ||||
| <ListGroupItemLabel as='span'>{children}</ListGroupItemLabel> | ||||
| </Flex> | ||||
| </Box> | ||||
| ); | ||||
| } | ||||
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.
When a keyboard user tabs through the enabled scope checkboxes,
focusRing={false}suppresses the focus styles fromcommon.borderVariants, while the input base uses a transparent outline and removes the native checkbox appearance. Since the surrounding label has no:focus-withinindicator, users cannot see which scope has focus; retain the standard input focus ring or add an equivalent visible indicator to the label.Useful? React with 👍 / 👎.