-
Notifications
You must be signed in to change notification settings - Fork 419
feat(timeline): Expose method to send galleries in matrix-sdk-ui #5125
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
Merged
Merged
Changes from 11 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
a5594c1
feat(timeline): Expose method to send galleries in matrix-sdk-ui
Johennes 5b0b56f
Add changelog
Johennes 7b6c0f0
Add missing cfg attributes
Johennes 793d147
Merge branch 'main' into johannes/msc4274-step3
Johennes 2ff4345
Introduce galleries module to prevent excessive use of cfg
Johennes f9f3f49
Switch to assert_pending
Johennes 84e36c4
Pass duration into assert_let_timeout instead of using sleep
Johennes c960200
Don't hide galleries in default event filter
Johennes 59b92df
Send a gallery event in integration test to exercise filtering
Johennes 434b896
Enable matrix-sdk-ui/unstable-msc4274 from matrix-sdk-ffi
Johennes f873500
Revert "Don't hide galleries in default event filter"
Johennes 2b0db6c
Merge branch 'main' into johannes/msc4274-step3
Johennes 80e2014
Revert "Revert "Don't hide galleries in default event filter""
Johennes 8328039
Mock media config requests
Johennes e9dfb45
Temporarily switch back to filtering out gallery events
Johennes 22f1991
Revert "Temporarily switch back to filtering out gallery events"
Johennes File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I think this should be true, so as to display galleries by default. Wouldn't this make the test you've added fail, since it'd filter out the gallery event from rendering into an item? Maybe that's why CI's failing now.
Uh oh!
There was an error while loading. Please reload this page.
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.
Oh, right! 🤦♂️
Have switched it around. It didn't seem to affect the test, however, which is mildly concerning. 🤔
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 test is likely not running; investigating.
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.
Hah:
Cargo.toml, so that it's enabled workspace-wide, and included in testing. I think the latter would be great, since the FFI layer likely will want it anyways.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.
Oh! 🤯
Ok, I hope I got all of this right. The test now does fail for me also locally (with the erroneous
false). When I switch back totrue, it succeeds.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.
Does it? Looks like it's the sending of the media gallery that fails, not the filtering out…
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.
Oh, you're right. 🤦♂️ Ok, will have to look into what's going on there now. 🤔
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.
Looks like the media config requests need mocking after merging in
main. Added those and switched back tofalseso the CI should now fail timing out waiting for the sent gallery.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.
Thanks, it seems to be failing as intended now: no item is pushed, while we expected one 🥳 Good job making the test meaningful and winning over CI!
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.
Lets hope it actually turns green now. 😅🤞