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

Experiment/whatsapp action UI mockup #40

Merged
merged 39 commits into from
Feb 10, 2025

Conversation

dmsynge
Copy link
Contributor

@dmsynge dmsynge commented Feb 3, 2025

This is a relatively large PR that effectively updates the whatsapp thread editing interface, and provides a better structure for components and code. The goals are to:

  1. improve the usability and intuitiveness of the interface; and
  2. make it more maintainable and easier to reason about

I think by-and-large it does that. However, there are four things that are worth noting:

  1. Overall, I wanted the drawer with the editing components for messages and templates to slide in and cover the right side of the screen (rather than the bottom), as I think that would be better for desktop, however, I wasn't able to find a way to do that. Not a big deal.
  2. I think using the Event dropdown popover component inside a drawer (which is implemented as a Modal by shadcn-svelte causes this bug sometimes: Dialog is capturing focus events and stopping them from being propogated huntabyte/bits-ui#755 I haven't been able to find a pattern with triggering it, but it's definitely annoying as it crashes the browser tab. I tried bumping the version of the underlying bits-ui library, but that didn't seem to help. I'm happy to push it as-is to production, but I will want to monitor it closely.
  3. The UI for editing templates isn't perfect for the text messages, because you don't see the message content when you're editing the fragments. I will want to make this inline in the future.
  4. There are no tests yet. I'm in the process of writing them, but I wanted to open this PR first to give you a chance to explore the code. I will deliver the tests though, and I've tried to make these components fairly testable.

This happened when installing the latest version of the DropdownMenu
component
This is the only part of the new system that uses an old comp. I will
probably refactor it, but for now it's working OK. I just wanted to add
a default message.
It needs to figure out what kind of message it is, and display the
content accordingly
This is only for displaying the buttons, not editing them or anything
like that. Therefore, it just needs to figure out if the message object
has buttons and to display them
This is the place that owns and  manages the state of the message thread
This small update brings the message display components into line with
WhatsApp messages
Because I ran the autogenerate script this also includes localization
for an unrelated "choose your language" localization
Based on the component to edit the action on a message, but underlying
data structure is slightly different. This isn't very DRY, but it may be
better to keep these components separate. We can revisit this if we fund
ourselves having to update them frequently
The UI for editing template tags isn't perfect, because when the drawer
slides out you're not able to see the message content. I think
eventually editing the text components inline will be a simpler
interface, but this is acceptable for now.
- Update imports to be new components
- Move most of the logic out to external /actions/*.ts files
- Add loading indicators and better error handling
- Add more consistent toast messages on success and error
We don't actually want to keep the interactive message parameters when
we're converting an interactive message into a type. Before this commit,
this function would convert the message type to "text" but also keep all
the interactive state, too. This doesn't cause an issue in our editing
screens, but would cause 400 errors if sent to the WhatsApp API
Copy link
Collaborator

@kennlebu kennlebu 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 amazing work. Very nice job. A lot better than what we had before. Thanks so much for working on this

@dmsynge dmsynge merged commit 6924d82 into main Feb 10, 2025
3 checks passed
kennlebu pushed a commit that referenced this pull request Feb 12, 2025
* Added shadcn DropdownMenu component

* Added an optional reset callback function for when the upload is deleted

* Adding ellipsis to our string clamp function

* Updating the page to use new WhatsApp thread composition components

* Add some extra AWS addresses to connect-src CSP directive for uploads

* Chore: version bump bits-ui

This happened when installing the latest version of the DropdownMenu
component

* Added default text to NewMessage creation

This is the only part of the new system that uses an old comp. I will
probably refactor it, but for now it's working OK. I just wanted to add
a default message.

* Simple component for displahying message content

It needs to figure out what kind of message it is, and display the
content accordingly

* Component for displaying message buttons

This is only for displaying the buttons, not editing them or anything
like that. Therefore, it just needs to figure out if the message object
has buttons and to display them

* Component that represents a single message

* Compnent to represent a list of messages in a thread

This is the place that owns and  manages the state of the message thread

* Main logic functions for WhatsApp message thread composition

* Editing frame for WhatsApp message content, using shadcn Drawer comp

* Main editing components for editing WhatsApp message content and structure

* Components for managing WhatsApp message actions

* Update the design of the message display components

This small update brings the message display components into line with
WhatsApp messages

* Update config

* Help fix some dev server instability

* Localizations for message creation

Because I ran the autogenerate script this also includes localization
for an unrelated "choose your language" localization

* Removed fetchMessages from messages because it's duplicated in threads.ts

* Functions to work with message components

* Functions to work with message threads

* WhatsApp message template selector

* Removed old component files

* Updated import to refrence new actions files

* Updated import to reference new actions files

* Added toast messages and loading effect on message save

* Version bumped bits-ui

* Component to render a template

* Component to edit the action on a template

Based on the component to edit the action on a message, but underlying
data structure is slightly different. This isn't very DRY, but it may be
better to keep these components separate. We can revisit this if we fund
ourselves having to update them frequently

* The drawer component for editing template content

The UI for editing template tags isn't perfect, because when the drawer
slides out you're not able to see the message content. I think
eventually editing the text components inline will be a simpler
interface, but this is acceptable for now.

* Core component for templates

* New version of component to create message

* New component to select the type of action for a button

* New component for adding actions to a template button

* Making the +page.svelte component work for new system

- Update imports to be new components
- Move most of the logic out to external /actions/*.ts files
- Add loading indicators and better error handling
- Add more consistent toast messages on success and error

* Refactor to avoid mutating function param state

* Removed error in message type conversion

We don't actually want to keep the interactive message parameters when
we're converting an interactive message into a type. Before this commit,
this function would convert the message type to "text" but also keep all
the interactive state, too. This doesn't cause an issue in our editing
screens, but would cause 400 errors if sent to the WhatsApp API

* Tests for WhatsApp message and template logic
Copy link

sentry-io bot commented Feb 17, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ TypeError: Cannot read properties of undefined (reading 'call') /(app)/communications/whatsapp/[thread_id] View Issue

Did you find this useful? React with a 👍 or 👎

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