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

Update the UI for the mailbox viewer #822

Merged
merged 3 commits into from
Oct 14, 2023
Merged

Conversation

dsincl12
Copy link
Contributor

@dsincl12 dsincl12 commented Oct 12, 2023

This will give the mailbox viewer a new UI.

Screenshot 2023-10-12 at 3 32 20 PM

Screenshot 2023-10-12 at 3 32 35 PM

Screenshot 2023-10-12 at 3 32 49 PM

@sweep-ai
Copy link

sweep-ai bot commented Oct 12, 2023

Apply Sweep Rules to your PR?

  • Apply: Leftover TODOs in the code should be handled.
  • Apply: All new business logic should have corresponding unit tests in the tests/ directory.
  • Apply: Any clearly inefficient or repeated code should be optimized or refactored.

@princemaple
Copy link
Member

Thanks for the PR! Could you add a a couple screenshots to the description? I'm currently travelling and may have some trouble reviewing code / visual. I can tell that you swapped outdated bootstrap for custom tailwind styles. Appreciate the effort. 🙏

@princemaple
Copy link
Member

Thank you! I'll give it a good scan tomorrow and probably merge it. 💜

Copy link
Member

@princemaple princemaple left a comment

Choose a reason for hiding this comment

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

Hi, PR is looking great in general. Just a few nits :) Cheers.

<h6 class="list-group-item-heading"><%= render_recipient(email.from) %></h6>
<p class="list-group-item-text"><%= email.subject %></p>
<a href="<%= to_absolute_url(@conn, id) %>" class="flex flex-col border-b border-gray-200 p-4<%= if @email && @email.headers["Message-ID"] == id, do: " bg-gray-100" %>">
<% {name, _email_address} = email.from %>
Copy link
Member

Choose a reason for hiding this comment

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

image

This would be empty in this case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, will fix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, actually render_value with show n/a for the empty string

lib/plug/templates/mailbox_viewer/index.html.eex Outdated Show resolved Hide resolved
lib/plug/templates/mailbox_viewer/index.html.eex Outdated Show resolved Hide resolved
@princemaple
Copy link
Member

@dsincl12 thanks for coming back to me so quickly. I'm happy to merge this now but will wait a bit to make a release. I intend to get to back to this to clean somethings up and play with the design a bit myself. I am busy the next 6 days or so, I will make a release maybe in a week. Cheers!

@princemaple princemaple merged commit 74eaf41 into swoosh:main Oct 14, 2023
12 checks passed
@princemaple
Copy link
Member

I haven't got the time to play with the new UI. I decided to release it so the community can help polish it. ;)

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