-
-
Notifications
You must be signed in to change notification settings - Fork 833
Tests for RoomCreate #9997
Tests for RoomCreate #9997
Conversation
138c922
to
9c7c5a4
Compare
9c7c5a4
to
dbb8bcc
Compare
dbb8bcc
to
251a387
Compare
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.
Two minor (definitely non-blocking) comments
const wrapper = getComponent(createEvent); | ||
expect(wrapper.getByText("Click here to see older messages.")).toHaveAttribute( | ||
"href", | ||
"https://matrix.to/#/old_room_id/tombstone_event_id", | ||
); |
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.
A very minor point that adds very little here, but I think the 'right' way (think I picked this up from the creator of RTL, not the docs, in this post here: KCD) is to render the component then use screen
for querying.
As I said, minimal gain here.
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.
Good suggestion, applied - thanks.
it("Opens the old room on click", () => { | ||
const wrapper = getComponent(createEvent); | ||
const link = wrapper.getByText("Click here to see older messages."); | ||
fireEvent.click(link); |
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.
What would you think to using userEvent
here instead? I'd say a larger gain to doing that than the previous suggestion, as userEvent fires a few more events (mouseover, hover etc.) than purely the click action so is a better model of user interaction. It would be imported and then used as userEvent.click(link)
so would read the same.
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.
Sounds good - I was not aware of this - thanks!
Prep for MSC3946
Part of element-hq/element-web#24323
This change is marked as an internal change (Task), so will not be included in the changelog.