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: add listing email response using notify button #155

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

pedrofp4444
Copy link
Member

No description provided.

@netlify
Copy link

netlify bot commented Mar 9, 2023

Deploy Preview for coderdojobraga-app ready!

Name Link
🔨 Latest commit 93fbcc3
🔍 Latest deploy log https://app.netlify.com/sites/coderdojobraga-app/deploys/6582d0cc2c836300071b1861
😎 Deploy Preview https://deploy-preview-155--coderdojobraga-app.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@pedrofp4444 pedrofp4444 self-assigned this Mar 9, 2023
@netlify
Copy link

netlify bot commented Mar 9, 2023

Deploy Preview for coderdojobraga-blog canceled.

Name Link
🔨 Latest commit 05c6be1
🔍 Latest deploy log https://app.netlify.com/sites/coderdojobraga-blog/deploys/6410e36c6c192f000815f33e

@netlify
Copy link

netlify bot commented Mar 9, 2023

Deploy Preview for coderdojobraga-web canceled.

Name Link
🔨 Latest commit 93fbcc3
🔍 Latest deploy log https://app.netlify.com/sites/coderdojobraga-web/deploys/6582d0cc58e6f10008853413

apps/app/pages/index.tsx Outdated Show resolved Hide resolved
apps/app/pages/index.tsx Outdated Show resolved Hide resolved
apps/app/pages/index.tsx Outdated Show resolved Hide resolved
apps/app/pages/index.tsx Outdated Show resolved Hide resolved
@pedrofp4444 pedrofp4444 marked this pull request as draft March 14, 2023 21:18
@MarioRodrigues10
Copy link
Member

Instead of using a modal, it may make more sense to create a route such as /admin/mailbox to display all sent emails, as is done in the Phoenix Framework.

@ruilopesm
Copy link
Member

@MarioRodrigues10 Why would a page be better?

@MarioRodrigues10
Copy link
Member

@MarioRodrigues10 Why would a page be better?

Unlike a modal, which is temporary, displaying sent emails on a route like /admin/mailbox would allow the admin to access them permanently and view them sorted by their send date.

@ruilopesm
Copy link
Member

Unlike a modal, which is temporary, displaying sent emails on a route like /admin/mailbox would allow the admin to access them permanently and view them sorted by their send date.

But how can shuriken access the sent emails? That's a capability not provided by bokken, hence the use of a modal (which is "temporary").

@MarioRodrigues10
Copy link
Member

Unlike a modal, which is temporary, displaying sent emails on a route like /admin/mailbox would allow the admin to access them permanently and view them sorted by their send date.

But how can shuriken access the sent emails? That's a capability not provided by bokken, hence the use of a modal (which is "temporary").

Access to the sent emails could be provided through Plug.Swoosh.MailboxPreview in Phoenix, and implementing this feature in Bokken would not require significant changes, making it a viable alternative to using a modal.

@ruilopesm
Copy link
Member

Documentation says:

The preview is also available as a JSON endpoint.
curl http://localhost:4000/dev/mailbox/json

And this is the way to go, since what you mention it's not directly suitable for shuriken.

@MarioRodrigues10
Copy link
Member

Documentation says:

The preview is also available as a JSON endpoint.
curl http://localhost:4000/dev/mailbox/json

And this is the way to go, since what you mention it's not directly suitable for shuriken.

I agree, perhaps we only need to modify an if statement in router.ex and add logic to restrict access to only organizers to allow access to the route.

@ruilopesm ruilopesm changed the title Add listing email response using notify button feat: add listing email response using notify button Aug 20, 2023
@ruilopesm ruilopesm added the app App related contributions label Aug 20, 2023
@pedrofp4444 pedrofp4444 marked this pull request as ready for review December 18, 2023 10:47
Copy link

netlify bot commented Dec 18, 2023

Deploy Preview for coderdojobraga-maintenance canceled.

Name Link
🔨 Latest commit 93fbcc3
🔍 Latest deploy log https://app.netlify.com/sites/coderdojobraga-maintenance/deploys/6582d0ccdec242000753ec2d

Copy link

netlify bot commented Dec 18, 2023

Deploy Preview for coderdojobraga-blog canceled.

Name Link
🔨 Latest commit 93fbcc3
🔍 Latest deploy log https://app.netlify.com/sites/coderdojobraga-blog/deploys/6582d0cc9a0a3f000841d5a0

@pedrofp4444 pedrofp4444 removed the request for review from ruioliveira02 December 18, 2023 10:55
Afonso-santos
Afonso-santos previously approved these changes Dec 18, 2023
Copy link
Contributor

@Afonso-santos Afonso-santos left a comment

Choose a reason for hiding this comment

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

🚀

return (
<Modal
title={modalTitle}
visible={visible}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
visible={visible}
open={visible}

visible is deprecated which will be removed in next the major version of AntDesign. Use open instead 🙏🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app App related contributions
Projects
Status: 👀 In review
Development

Successfully merging this pull request may close these issues.

5 participants