Skip to content

UI/comment threads fixes#961

Merged
ButteryCrumpet merged 10 commits intomasterfrom
ui/comment-threads-fixes
May 25, 2021
Merged

UI/comment threads fixes#961
ButteryCrumpet merged 10 commits intomasterfrom
ui/comment-threads-fixes

Conversation

@ButteryCrumpet
Copy link
Copy Markdown
Contributor

FIxes

  • Correctly display user icon
  • sort by latest reply
  • Correctly set Closed threads to outdated
  • Add thread context summary to thread list

@ButteryCrumpet ButteryCrumpet force-pushed the ui/comment-threads-fixes branch from 08718dd to 8d3a0f8 Compare May 24, 2021 02:45
Comment thread src/shared/lib/injectors.tsx Outdated
import { SerializedUser } from '../../cloud/interfaces/db/user'
import { usePage } from '../../cloud/lib/stores/pageStore'

export function withLiveUser<
Copy link
Copy Markdown
Member

@Rokt33r Rokt33r May 25, 2021

Choose a reason for hiding this comment

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

We can make a hook useLiveUser which using the same hooks of withLiveUser
So I would like to use the below code.

export function useLiveUser(user: User) {
    const { permissions } = usePage()

    const normalizedUser = useMemo(() => {
      if (permissions == null) {
        return user
      }

      const permission = permissions.find((permission) => permission.user.id)
      return permission != null ? permission.user : user
    }, [user, permissions])
}

const LiveUserIcon = (props: Exclude<UserIconProps, 'user'>) => {
  const liveUser = useLiveUser()

   return <UserIcon {...props} user={liveUser}/>
}

By doing this, we don't have to introduce another pattern HOC.
And the hook pattern is much easier to compose with other hooks while composing HOCs are quite difficult to understand and control.)

@ButteryCrumpet ButteryCrumpet merged commit f1c4b5f into master May 25, 2021
@ButteryCrumpet ButteryCrumpet deleted the ui/comment-threads-fixes branch May 25, 2021 03:13
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.

2 participants