Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Populate the file panel using the event index if available. #3858

Merged
merged 23 commits into from
Jan 24, 2020

Conversation

poljar
Copy link
Contributor

@poljar poljar commented Jan 17, 2020

As the title says, this pull request implements pulling events containing files out of the event index and populating the file panel with them.

This intends to fix element-hq/element-web#4959.

A demonstration of this can be found here.

The remaining issue for now is that live events aren't added to the file panel while the panel is open. If the panel is reopened the newly arrived events are added to the top instead of the bottom.

I suspect we want to fix both of those at the same time while we add the ability for riot-web, which doesn't have event indexing, to show at least events in the live timeline in the file panel.

The main issue with that seems to be that this filter isn't smart enough to understand decrypted events that contain URLs.

Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

seems generally sane - mostly comments about codestyle which our linter doesn't cover.

src/Lifecycle.js Show resolved Hide resolved
src/components/structures/FilePanel.js Outdated Show resolved Hide resolved
src/components/structures/FilePanel.js Outdated Show resolved Hide resolved
src/indexing/EventIndex.js Outdated Show resolved Hide resolved
src/indexing/EventIndex.js Outdated Show resolved Hide resolved
src/indexing/EventIndex.js Outdated Show resolved Hide resolved
src/indexing/EventIndex.js Outdated Show resolved Hide resolved
src/indexing/EventIndex.js Outdated Show resolved Hide resolved
src/indexing/EventIndex.js Outdated Show resolved Hide resolved
src/indexing/EventIndex.js Outdated Show resolved Hide resolved
@poljar
Copy link
Contributor Author

poljar commented Jan 21, 2020

Updated now. Live events handling is still missing.

My plan is to add listeners for Room.timeline and Room.decrypted to add the live events to the loaded FilePanel.

If someone has a better idea, please shout.

@poljar poljar requested a review from a team January 21, 2020 16:11
@jryans jryans requested review from turt2live and removed request for a team January 21, 2020 16:56
@poljar
Copy link
Contributor Author

poljar commented Jan 22, 2020

This has been now updated to correctly handle the remaining problematic cases:

  • New events that arrive while the panel is closed are correctly added when the panel is re-opened.
  • New events that arrive while the panel is open.

Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

looks good to merge after the last 2 lint things. Thanks for working on this, and apologies for the delay in review.

ftr I have no idea how any of this is supposed to work, but it looks like its in the right places and does roughly the right thing. Very much assuming that bugs will show up over time and we can fix them as we see them.

src/indexing/EventIndex.js Outdated Show resolved Hide resolved
src/indexing/EventIndex.js Outdated Show resolved Hide resolved
@poljar
Copy link
Contributor Author

poljar commented Jan 24, 2020

ftr I have no idea how any of this is supposed to work, but it looks like its in the right places and does roughly the right thing. Very much assuming that bugs will show up over time and we can fix them as we see them.

Let me add a comment in that case to explain what it does.

@poljar poljar merged commit 37f289b into develop Jan 24, 2020
@poljar poljar deleted the poljar/seshat-filepanel branch June 21, 2020 17:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'Files' view in e2e-encrypted rooms does not show files
2 participants