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.

Sign up for GitHub

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

Jump to bottom

Change home timeline to reload after follow recommendations in web UI #16160

Merged
merged 1 commit into from
May 7, 2021
Merged

Change home timeline to reload after follow recommendations in web UI #16160

merged 1 commit into from
May 7, 2021

Conversation

Gargron
Copy link
Member

@Gargron Gargron commented May 5, 2021

No description provided.

@Gargron Gargron force-pushed the fix-empty-home-before-first-follow branch from 1d9b510 to 4baa163 Compare May 5, 2021 21:37
ClearlyClaire
ClearlyClaire requested changes May 5, 2021
View reviewed changes
Copy link
Contributor

@ClearlyClaire ClearlyClaire left a comment

Choose a reason for hiding this comment

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

I am not too happy with this change, it seems like it may cause more issues than it might help. In particular, reloading the page when the home TL is not in focus might cause issues.

Maybe a slightly hackish but less disruptive alternative would be to expand the home timeline when closing the onboarding component.

app/javascript/mastodon/features/home_timeline/index.js Outdated Show resolved Hide resolved
@Gargron Gargron force-pushed the fix-empty-home-before-first-follow branch from 4baa163 to 2fd5940 Compare May 5, 2021 23:43
@Gargron Gargron requested a review from ClearlyClaire May 6, 2021 00:11
@Gargron Gargron changed the title Change home timeline to load only when component is mounted in web UI Change home timeline to reload after follow recommendations in web UI May 6, 2021
ClearlyClaire
ClearlyClaire requested changes May 6, 2021
View reviewed changes
app/javascript/mastodon/features/ui/index.js
Comment on lines -367 to +366

setTimeout(() => this.props.dispatch(fetchMarkers()), 500);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why delay this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just felt there were too many requests issued at exactly the same time

500ms is barely noticable for a user but may reduce load on the server

Copy link
Contributor

Choose a reason for hiding this comment

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

if anything i think it would be best if we ensured this request was done before the notifications one, especially on the advanced layout, in order to avoid the unread markers popping “late”

app/javascript/mastodon/features/follow_recommendations/index.js Show resolved Hide resolved
@Gargron Gargron force-pushed the fix-empty-home-before-first-follow branch from 2fd5940 to 1d3d9d3 Compare May 6, 2021 13:44
@Gargron Gargron force-pushed the fix-empty-home-before-first-follow branch from 1d3d9d3 to 0f11b7c Compare May 6, 2021 13:53
@Gargron Gargron requested a review from ClearlyClaire May 6, 2021 23:43
ClearlyClaire
ClearlyClaire approved these changes May 7, 2021
View reviewed changes
Copy link
Contributor

@ClearlyClaire ClearlyClaire left a comment

Choose a reason for hiding this comment

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

I still have reservations about the unrelated timing changes, but otherwise it seems fine to me.

@Gargron Gargron merged commit 0ad240c into main May 7, 2021
@Gargron Gargron deleted the fix-empty-home-before-first-follow branch May 7, 2021 12:34
Gargron added a commit that referenced this pull request May 11, 2021
@Gargron
Fix error when rendering actor with hashtags in bio
e601257 
Fix #16217, regression from #16160
@Gargron Gargron mentioned this pull request May 11, 2021
Merged
Gargron added a commit that referenced this pull request May 11, 2021
@Gargron
Fix error when rendering actor with hashtags in bio (#16218)
be353bc 
Fix #16217, regression from #16160
chrisguida pushed a commit to Start9Labs/mastodon that referenced this pull request Feb 26, 2022
@Gargron @chrisguida
Change home timeline to reload after follow recommendations in web UI (…
ca694ff 
…mastodon#16160)
chrisguida pushed a commit to Start9Labs/mastodon that referenced this pull request Feb 26, 2022
@Gargron @chrisguida
Fix error when rendering actor with hashtags in bio (mastodon#16218)
bad8beb 
Fix mastodon#16217, regression from mastodon#16160
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ClearlyClaire ClearlyClaire ClearlyClaire approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Gargron @ClearlyClaire