Remove Text from RSP DropZone API #6152
Conversation
|
Build successful! 🎉 |
| @@ -41,13 +42,15 @@ function DropZone(props: SpectrumDropZoneProps, ref: DOMRef<HTMLDivElement>) { | |||
| let {styleProps} = useStyleProps(props); | |||
| let domRef = useDOMRef(ref); | |||
| let messageId = useId(); | |||
| let headingId = useId(); | |||
There was a problem hiding this comment.
Reid suggested to pass the headingId via slot which I would have liked to do but the problem is that it gets overridden in IllustratedMessage so it doesn't actually get passed all the way through
There was a problem hiding this comment.
I wonder why we decided to clear slots in Illustrated Message. Maybe we could test to see what happens when we remove that https://github.com/adobe/react-spectrum/pull/3420/files#diff-75401696e93859335f6081190bc047f0be8ff0b8e70f4639ae0e0944f8a3cf46R48-R52
There was a problem hiding this comment.
Yeah I tired removing it locally which seems to work, though it looks like the reason for adding <ClearSlots was because we were concerned of possible collisions since IllustratedMessage uses the "illustration" slot and so does anything that's wrapped with <Illustration>
packages/@react-spectrum/illustratedmessage/src/IllustratedMessage.tsx
Outdated
Show resolved
Hide resolved
|
Build successful! 🎉 |
|
Build successful! 🎉 |
| let ariaLabel = props['aria-label'] || stringFormatter.format('dropzoneLabel'); | ||
| let messageId = props['aria-labelledby']; | ||
| // Chrome + VO will not announce the drop zone's accessible name if useLabels combines an aria-label and aria-labelledby |
There was a problem hiding this comment.
apparently this doesn't seem to be an issue anymore? seems like chrome + VO is now happy to announce the accessible name if useLabel combines aria-label and aria-labelledby so i've removed the workaround and updated the tests accordingly
There was a problem hiding this comment.
we might want to bisect chrome a bit so we don't remove while a recent-ish version still has the issue
can you do that? https://github.com/jay0lee/chrome-bisect
There was a problem hiding this comment.
okay...tried the chrome bisect and tested back to v111 which was released march 2023 and for whatever reason could not reproduce it. im not sure what's going on cause i swear this was an issue in the past 😭
yihuiliao
changed the title
|
Build successful! 🎉 |
|
Build successful! 🎉 |
|
Build successful! 🎉 |
|
## API Changes
unknown top level export { type: 'any' } |
Currently if you want to use RSP DropZone, you also have to import from RAC in order to use the Text component which is needed to create an accessible name for screenreader users. This might be confusing for some users which is why we'd like to get rid it of it and have
<Heading>handle that work instead.With this change, the API should look something like this:
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: