Skip to content
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

Refactor to use TimelineView #117

Merged
merged 6 commits into from
Nov 1, 2022

Conversation

MadLittleMods
Copy link
Contributor

@MadLittleMods MadLittleMods commented Nov 1, 2022

Refactor to use TimelineView. We have to figure out our own layout but does get rid of some of the RoomView boilerplate. The TimelineView still has outside boilerplate styles to work.

There shouldn't be any visible change:

environment Before After
desktop
mobile (right-panel collapsed)
mobile (right-panel expanded)

Why are we switching from RoomView to TimelineView

I continue to suspect that using TimelineView rather than RoomView would make more sense for your use case. In the future, RoomView will indeed evolve, but possibly in ways that don't simplify things for your use case. For example, we'll soon add support to show video calls in RoomView. We might add widget support some day. All things I imagine you have no need for. Because ... what you want to show is the timeline. The fact that you need to replace the room header already shows your needs are not exactly the same as a chat client.
IMO, having to implement the layout between the timeline, header and banner yourself is outweighed by your current need to mock the room view model completely, in ways that are bound to break in the future as the room UI allows more and more interactions not related to the timeline.

-- @bwindels, element-hq/hydrogen-web#864 (comment)

Also aligns with what Chatterbox has to do SDK wise. Our layout aligns more with Hydrogen which is why we originally used RoomView though.

Also a soft pre-requisite for #114 so we can more easily pass options to TimelineView to change how the scroll works. Although we could work around and pass options through RoomView if necessary.

Todo

  • Make sure layout is the same on desktop
  • Make sure layout is the same on mobile
  • Make sure right panel expands and collapses and URL changes
  • Make sure calendar updates as you scroll the timeline
  • Make sure disabled composer date updates as you scroll the timeline
  • Make sure the page loads with the timeline scrolled to the bottom
    • => Nevermind, this seems to be an existing problem as I can reproduce on main and previous revisions
    • Bugged behavior:
    • Seems to be offset by the height of the header (.Timeline_scroller, $0.scrollHeight - $0.scrollTop - $0.clientHeight = 56px)
    • Doesn't occur on page refresh, only on a clean page load

@MadLittleMods MadLittleMods added T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. A-archive-room-view The view to look at a room day by day in the archive labels Nov 1, 2022
@MadLittleMods MadLittleMods marked this pull request as ready for review November 1, 2022 13:35
@MadLittleMods MadLittleMods merged commit 718f01e into main Nov 1, 2022
@MadLittleMods MadLittleMods deleted the madlittlemods/refactor-to-timelineview branch April 26, 2023 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-archive-room-view The view to look at a room day by day in the archive T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant