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

Build message threads #3593

Merged
merged 3 commits into from
Jan 24, 2024
Merged

Build message threads #3593

merged 3 commits into from
Jan 24, 2024

Conversation

thomtrp
Copy link
Contributor

@thomtrp thomtrp commented Jan 23, 2024

Adding logic to fill the right drawer with all messages:

  • all messages design are looking as the figma. Not sure this is pixel perfect, but this is a first version
  • messages can be opened and closed, it is only missing a smooth transition
  • opened messages are stored in state. Which means that they stay open when the right drawer is closed
  • clicking on another email thread will keep the right drawer open and only change the content

In future PRs:

  • using real call instead of mocks
  • do a pass on the design
  • add tests
Enregistrement.de.l.ecran.2024-01-24.a.10.56.42.mov

} else {
setIsOpenedMessage(id);
}
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I build a hook useThreadMessage ? Or is it acceptable to keep that logic in the component?

@thomtrp thomtrp marked this pull request as ready for review January 24, 2024 10:20

import { MockedThread } from '@/activities/emails/mocks/mockedThreads';

export const viewableThreadState = atom<MockedThread | null>({
Copy link
Member

Choose a reason for hiding this comment

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

I'm very surprise about the typing getting out of the mocks!

Copy link
Member

Choose a reason for hiding this comment

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

Also, I think this state is not useful, we should rely on apollo and not on the cache.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. But I will test it once I do not have to mock the data anymore. For now I am not performing any query

Copy link
Contributor Author

Choose a reason for hiding this comment

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

About the typing, this is something you want to discuss with Coco and Raphael to redefine the API?

@thomtrp thomtrp merged commit e85f65a into main Jan 24, 2024
13 checks passed
@thomtrp thomtrp deleted the tt-build-message-threads branch January 24, 2024 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants