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

feat: redirect to first conversation in list #1036

Merged
merged 11 commits into from
Nov 2, 2022

Conversation

neekolas
Copy link
Contributor

@neekolas neekolas commented Oct 31, 2022

What does this PR do?

  • Attempts to redirect to the first conversation in the list
  • Conditions to allow a redirect are
  1. The user's viewport is 1024px or greater (we don't want this behaviour on mobile, since we use the /messages as a distinct route to show the conversation list)
  2. The user is on the /messages route
  3. There is at least one conversation in the list loaded.

I think this feature is going to get picked up in a planned refactor to merge the Message.tsx and index.tsx pages into a single route. Right now, the necessary information to execute this redirect is lower in the React tree than is ideal.

I also had to add a new React hook to track the window width to avoid this behaviour on mobile, which is unfortunate. Open to other solutions that don't rely on window.innerWidth.

The PR also changes the send button to only icon in the mobile view and makes the mobile view flow start from 1024px

Fixes # (issue)

CleanShot.2022-10-31.at.10.47.47.mp4

image

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Enhancement (non-breaking small changes to existing functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

  • Go to the /messages page on the desktop. The first conversation should be automatically selected once loading is complete
  • Go to the /messages page on mobile. The conversation list should appear, and it should be possible to navigate to the individual conversation and back.
  • In mobile view the send button should only have an icon

@neekolas neekolas requested a review from bigint as a code owner October 31, 2022 17:49
@vercel
Copy link

vercel bot commented Oct 31, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
lenster ✅ Ready (Inspect) Visit Preview Nov 1, 2022 at 7:29PM (UTC)

@vercel vercel bot temporarily deployed to Preview October 31, 2022 17:52 Inactive
@bigint bigint changed the title Redirect to first conversation in list feat: redirect to first conversation in list Oct 31, 2022
@neekolas neekolas changed the title feat: redirect to first conversation in list Redirect to first conversation in list Oct 31, 2022
@neekolas neekolas changed the title Redirect to first conversation in list redirect to first conversation in list Oct 31, 2022
@vercel vercel bot temporarily deployed to Preview October 31, 2022 18:29 Inactive
@neekolas neekolas changed the title redirect to first conversation in list feat: redirect to first conversation in list Oct 31, 2022
Copy link
Contributor

@bhavya2611 bhavya2611 left a comment

Choose a reason for hiding this comment

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

Small comment

src/components/Messages/PreviewList.tsx Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview October 31, 2022 20:20 Inactive
@bigint
Copy link
Member

bigint commented Nov 1, 2022

Can you please resolve the conflicts 🙏🏼

…o-select-first-conversation

Conflicts:
src/components/Messages/Message.tsx
@bhavya2611
Copy link
Contributor

@bigint resolved conflicts

@vercel vercel bot temporarily deployed to Preview November 1, 2022 08:29 Inactive
}

// Borrowed from https://usehooks.com/useWindowSize/
const useWindowSize = (): Size => {
Copy link
Member

Choose a reason for hiding this comment

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

We can use tailwind's classes instead of this hook right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For styling, absolutely. But this is used to change behavior

@vercel vercel bot temporarily deployed to Preview November 1, 2022 19:29 Inactive
@bigint bigint merged commit 23bf2a7 into main Nov 2, 2022
@bigint bigint deleted the auto-select-first-conversation branch November 2, 2022 04:27
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