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

Render more while reloading only some resources #2480

Merged
merged 16 commits into from
May 22, 2024

Conversation

matc-pub
Copy link
Collaborator

This change can keep components mounted when the next URL matches the current
route. This allows components to keep rendering some resources while reloading
others.

Commit ff5a391 contains most of the markup changes without running prettier, the follow up commit is only running prettier.

Changes

Home, Community

  • Sidebar stays loaded while reloading posts/comments
  • Community name stays visible while reloading posts/comments
  • Community initially displays community name from URL
  • HomeCommunity.webm (Side by side, left old, right new)

Post

  • Post stays loaded while reloading comments.
  • Comment listing type is reflected in URL
  • Comment link changed to /post/:post_id/:comment_id? (/comment/:comment_id still works)
  • Comment link doesn't change listing type, doesn't reload post
  • Same for "Show context" and "View all comments" buttons
  • Flat comments view works with New and Old, sorted by published
  • Post.webm

Search

  • Doesn't reload Community/User filter while updating search results
  • Search.webm

Modlog

  • Doesn't reload community name when changing filters
  • Modlog.webm

Profile

  • Keeps showing user info while reloading data
  • Overview, Comments and Posts (are the same request) don't reload data when switching between them
  • Profile.webm

Spinners

  • Communities shows spinner below buttons
  • Registration Applications shows spinner below buttons
  • Create Post no longer shows spinner
  • spinner.webm

Prompt

  • warns before reloading posts/comments when editing post/comment
  • prompts.webm

Inbox

  • Fixed issue with high latency responses (this is the only instance where this already was kind of an issue)
  • highLatency.webm (dev tools set to 3s latency)

Other changes

  • Move most of componentDidMount to componentWillMount, this avoids updating component states immediately after the first render
  • Include children of auth and anonymous guard in first render.
  • Fix amAdmin check in reports fetchInitialData
  • AdminSettings remove unused currentTab state

Copy link
Member

@SleeplessOne1917 SleeplessOne1917 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 a massive improvement to UX. I need clarification on some changes, but otherwise this is great.

Comment on lines 18 to 22
return (
this.state.ellipses || (
<span dangerouslySetInnerHTML={{ __html: "&nbsp;" }} />
)
);
Copy link
Member

Choose a reason for hiding this comment

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

Why the space is ellipses doesn't exist? And why use dangerouslySetInnerHTML for this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Modlog title collapses when it cycles to the empty string

Looks like <span>&nbsp;</span> does work. Thought i tried that. I'm moving it to modlog directly.

@@ -64,7 +65,7 @@ export function SubscribeButton({
>
{I18NextService.i18n.t("subscribe")}
</button>
<RemoteFetchModal communityActorId={actor_id} />
<RemoteFetchModal show={false} communityActorId={actor_id} />
Copy link
Member

Choose a reason for hiding this comment

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

Why is show hard-coded to false?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Going to make it optional in modalMixin.

@SleeplessOne1917 SleeplessOne1917 merged commit b7fe70d into LemmyNet:main May 22, 2024
1 check passed
@dessalines
Copy link
Member

@matc-pub would you be willing to help with lemmy-ui PRs, and be a codeowner? It'd be much appreciated.

@matc-pub
Copy link
Collaborator Author

matc-pub commented Jun 4, 2024

I can give it a try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants