Skip to content

chore: add test to ensure it is possible to trigger native event - #1114

Merged
mdjastrzebski merged 1 commit into
callstack:mainfrom
AugustinLF:add-test-covering-native-events
Sep 15, 2022
Merged

chore: add test to ensure it is possible to trigger native event#1114
mdjastrzebski merged 1 commit into
callstack:mainfrom
AugustinLF:add-test-covering-native-events

Conversation

@AugustinLF

Copy link
Copy Markdown
Collaborator

This PR adds tests for a behaviour we support (triggering event that don't exist on the JS side but are handled natively). It is however not tested, so I want to add test to make sure we don't lose this feature by mistake (see discussions in #1080 (comment)).


// Those events ideally should be triggered through `fireEvent.scroll`, but they are handled at the
// native level, so we need to support manually triggering them
describe('native events', () => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
describe('native events', () => {
describe('native event support', () => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if we should actually use host component event support instead, as by design we do not support "native" events.

Comment thread src/__tests__/fireEvent.test.tsx

@mdjastrzebski mdjastrzebski left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good!

@mdjastrzebski mdjastrzebski changed the title Add test to ensure it is possible to trigger native event chore: add test to ensure it is possible to trigger native event Sep 15, 2022
@mdjastrzebski
mdjastrzebski merged commit f0a1602 into callstack:main Sep 15, 2022
@AugustinLF
AugustinLF deleted the add-test-covering-native-events branch September 15, 2022 08:52
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