-
-
Notifications
You must be signed in to change notification settings - Fork 829
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
Implement sticky date separators #1353
Conversation
Use `react-sticky` to implement sticky date separators. This will pin a date separator to the top of the timeline panel when the separator scrolls out of the top of the view. A known issue of this is that the spinner, which is in line with event tiles in the timeline, will appear to push the stuck date separator down. In reality the first date separator after the spinner is in line with event tiles and is not stuck because the spinner forces the timeline to be scrolled slightly further down than it would be otherwise. But also, date separators in the timeline (not "stuck") have a greater height. Ideally the date separator would be suppressed whilst back paginating, but this will cause the stuck separator to flicker on and off. This is why the suppression has been removed.
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.
Hmm, lots of stuff in this PR and not easy to see what's actually going on. I think this is fine other than comments though.
}); | ||
} | ||
}, | ||
|
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.
Hmm, you've done this now but we really should be using the i18n for date formatting: https://github.com/martinandert/counterpart#localization
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 cool, I'll do that instead
var sn = this._getScrollNode(); | ||
debuglog("Scroll event: offset now:", sn.scrollTop, | ||
"_lastSetScroll:", this._lastSetScroll); | ||
|
||
this.node = sn; |
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.
unused?
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.
Actually no, this is why any of the sticky stuff works and I 100% should have commented as such.
Should I split it up into several commits? I realise that it's all been done in one blob but need not go down in history as such. |
Use
react-sticky
to implement sticky date separators. This will pin a date separator to the top of the timeline panel when the separator scrolls out of the top of the view.A known issue of this is that the spinner, which is in line with event tiles in the timeline, will appear to push the stuck date separator down. In reality the first date separator after the spinner is in line with event tiles and is not stuck because the spinner forces the timeline to be scrolled slightly further down than it would be otherwise. But also, date separators in the timeline (not "stuck") have a greater height.
Ideally the date separator would be suppressed whilst back paginating, but this will cause the stuck separator to flicker on and off. This is why the suppression has been removed.
Also,
DateUtils.formatDateSeparator
was factored out ofDateSeparator
and fixed so that it will now display:codep element-hq/element-web#4939