Skip to content

Conversation

gribnoysup
Copy link
Collaborator

@gribnoysup gribnoysup commented Jun 23, 2022

After spending way too much time debugging this latest issue that Cloud ran into while updating our packages to latest in their codebase we finally pinpointed it to the difference in the react-aria dependency versions. There was a subtle change in the package behavior that unfortunately started to cause issues in the <Document /> component. We were not affected yet because package-lock guarded us from encountering this, but as it's affecting Cloud and we would need to update eventually it made sense to me to update and address the issue right away.

So the root cause was that instead of triggerProps having no onFocus and onBlur properties at all, react-aria started to explicitly pass these properties as undefined in some cases which caused the spreading to override our onFocus and onBlur callbacks that we were passing to the component. This in turn meant that editor.start / stop was not called on the hadron doc anymore leading to the same issue we fixed in #2942

This patch makes sure that we are merging trigger props in a way that always makes sure that our event callbacks will get passed to the component. There are no new tests added in this PR because it was already covered by existing ones and updating the deps without adding the fix was catching it (also chore and not fix because nothing is really broken in Compass)

COMPASS-5917

@gribnoysup gribnoysup force-pushed the compass-5917-fix-react-aria-on-focus-blur branch from 778a5dc to f99d586 Compare June 23, 2022 17:28
Copy link
Contributor

@lerouxb lerouxb left a comment

Choose a reason for hiding this comment

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

oof

@gribnoysup gribnoysup merged commit d2e71b0 into main Jun 24, 2022
@gribnoysup gribnoysup deleted the compass-5917-fix-react-aria-on-focus-blur branch June 24, 2022 08:59
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.

2 participants