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

make the lifetimes of the RM configurable #3450

Merged
merged 3 commits into from
Sep 17, 2019
Merged

Conversation

ara4n
Copy link
Member

@ara4n ara4n commented Sep 17, 2019

based on lots of FTUE feedback from Mozilla

readMarkerInViewThresholdMs: SettingsStore.getValue("readMarkerInViewThresholdMs"),

// how long to show the RM for when it's scrolled off-screen
readMarkerOutOfViewThresholdMs: SettingsStore.getValue("readMarkerOutOfViewThresholdMs"),
Copy link
Member

Choose a reason for hiding this comment

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

these won't update without switching between rooms

Copy link
Member Author

Choose a reason for hiding this comment

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

correct. does i wonder if this matters though? it's what we do for 12h timestamps etc too.

Copy link
Member

Choose a reason for hiding this comment

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

well, we should be better at these sorts of things. There's bugs all over the place for "X does not update after changing the setting". They weren't problems before because the timeline would remount after the settings page went away, but now we're a dialog and that causes a whole set of problems.

src/i18n/strings/en_EN.json Outdated Show resolved Hide resolved
@ara4n ara4n requested a review from a team September 17, 2019 16:52
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.

This is fine for now. Solving the realtime updates problem sounds like something that should be coordinated across all the setting flags anyhow, instead of being done piecemeal (and differing).

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.

2 participants