-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
assorted relationship field fixes #5855
Conversation
🦋 Changeset detectedLatest commit: 8913c74 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/keystonejs/keystone-next-docs/3RERvtcXWdpiqVyHD3hFGCgvaqPm |
826a003
to
d5c267c
Compare
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
5b7f0db
to
6b8c5d2
Compare
marginY="medium" | ||
across | ||
css={{ | ||
width: '100%', |
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.
Same fix as in the images and file field.
We don't have a good way to individually alter the flex styles of children since each child is programatically wrapped in an element, so we have to do it here.
@@ -113,7 +113,7 @@ export const Field = ({ | |||
|
|||
if (value.kind === 'cards-view') { | |||
return ( | |||
<Stack as="fieldset" gap="medium"> | |||
<FieldContainer as="fieldset"> |
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.
the legend
element expects to have a fieldset
element as its immediate parent.
the Stack
component negates this via its inherent behaviour (wrapping each child in a pre-styled element)
@@ -143,7 +143,7 @@ export const Field = ({ | |||
} | |||
|
|||
return ( | |||
<FieldContainer> | |||
<FieldContainer as="fieldset"> |
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.
see above
packages-next/fields/src/types/relationship/views/cards/index.tsx
Outdated
Show resolved
Hide resolved
packages-next/fields/src/types/relationship/views/cards/index.tsx
Outdated
Show resolved
Hide resolved
} | ||
itemForField[graphqlField] = fieldGetter.data; | ||
} | ||
<Stack |
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.
We only want to wrap the list items in a ul
, the actions for creating a new item / inline connection to an existing item should sit outside of this list.
…ystonejs/keystone into a11y/relationship-field-label
const itemGetter = items[id]; | ||
const isEditMode = !!(onChange !== undefined) && value.itemsBeingEdited.has(id); | ||
return ( | ||
<CardContainer role="status" mode={isEditMode ? 'edit' : 'view'}> |
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.
persistent card container now so we can append a role status, and announce when a card enters edit mode and vice versa.
div
FieldContainer withfieldset
elementedit
mode.New screen reader experience below:
Uploading relationship-field-updates.mov…