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

Centralize auth messages #269

Merged
merged 7 commits into from
Feb 3, 2024

Conversation

josefarias
Copy link
Contributor

@josefarias josefarias commented Feb 3, 2024

Closes #264

At first, I thought by "centralize" we meant we wanted all the copy for the messages in a single place for easy editing. So I added rudimentary i18n, as well as i18n-tasks, which helps keep i18n files tidy. (That's on me for not paying attention, sorry I jumped the gun here)

Then I realized we actually wanted the messages to all appear in a single place for auth-related forms, so I pulled out a helper for that, too.

Let me know if we don't want the i18n part of this. Happy to take it out.

To be clear, there's still plenty of i18n work left to do — this was more meant to organize the auth messages. The rest of the app needs to be configured still. We can tackle that work as part of #256.

@Shpigford
Copy link
Member

@josefarias Couple of conflicts that popped up from a PR merge this morning.

@Shpigford Shpigford merged commit c5192ee into maybe-finance:main Feb 3, 2024
4 checks passed
JuanVqz pushed a commit to JuanVqz/maybe that referenced this pull request Feb 5, 2024
* Add i18n-tasks

* Add auth-related i18n

* Centralize auth messages

* Remove safe navigation

* Revert "Remove safe navigation"

This reverts commit 56b5e01.

* Remove newline in Gemfile
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.

Centralize auth errors/notices/alerts
2 participants