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.

Sign up for GitHub

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

Jump to bottom

Add symfonycasts/[email protected] template #53

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

Add symfonycasts/[email protected] template #53

wants to merge 33 commits into from

Conversation

cavasinf
Copy link
Collaborator

@cavasinf cavasinf commented Jan 10, 2022 •
edited
Loading

Description

From #10

More here: https://github.com/SymfonyCasts/reset-password-bundle

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I updated the documentation (see here)
  • I agree that this code will be published under the MIT license

@cavasinf cavasinf added Feature Feature requested Status: Needs Review Not under investigation labels Jan 10, 2022
@cavasinf cavasinf linked an issue Jan 10, 2022 that may be closed by this pull request
Though on symfonycasts/reset-password-bundle #10
Open
@cavasinf
Copy link
Collaborator Author

@kevinpapst How do you generate your trads file in the Bundle ?

php bin/console translation:extract --force fr ?

@cavasinf
Copy link
Collaborator Author

@kevinpapst How do you generate your trads file in the Bundle ?

php bin/console translation:extract --force fr ?

@kevinpapst Any answer of that?

@kevinpapst
Copy link
Owner

Never used that command I guess. I always create translation files while developing views.

@cavasinf cavasinf added Status: Needs Work Need to be reworked and removed Status: Needs Review Not under investigation labels Jan 14, 2022
@cavasinf
Copy link
Collaborator Author

Missing translations !

@cavasinf cavasinf added Status: Needs Review Not under investigation and removed Status: Needs Work Need to be reworked labels Jan 31, 2022
cavasinf
cavasinf commented Feb 1, 2022
View reviewed changes
Copy link
Collaborator Author

@cavasinf cavasinf left a comment

Choose a reason for hiding this comment

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

Added FR translation.

@kevinpapst It is ready for me, do you want to add the German translation ?

@cavasinf
Copy link
Collaborator Author

cavasinf commented Jan 5, 2023

All set and ready to merge.
ATE, only template and translations are added 👍

@cavasinf cavasinf requested a review from kevinpapst January 5, 2023 13:44
@cavasinf cavasinf added Status: Reviewed Has staff reply/investigation and removed Status: Needs Review Not under investigation labels Jan 5, 2023
kevinpapst
kevinpapst requested changes Jan 5, 2023
View reviewed changes
Copy link
Owner

@kevinpapst kevinpapst left a comment

Choose a reason for hiding this comment

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

  1. Can you link the docs page from the docs index
  2. _layout.html.twig does not match any filename convention?
  3. check_email.html.twig should be check-email.html.twig becuase all files included directly by the controller should be x-y.html.twig - correct?
  4. Add a composer suggest for the package

@kevinpapst
Copy link
Owner

Can you please include these german / DE translations:

      <trans-unit id="948fezF"  resname="Password Reset Email Sent">
        <source>Password Reset Email Sent</source>
        <target>E-Mail für die Zurücksetzung des Passwort wurde gesendet</target>
      </trans-unit>
      <trans-unit id="5FE896d" resname="Enter your email address and your password will be reset and emailed to you">
        <source>Enter your email address and your password will be reset and emailed to you</source>
        <target>Geben Sie Ihre E-Mail Adresse an und Sie bekommen eine Nachricht um Ihr Passwort zurückzusetzen</target>
      </trans-unit>
      <trans-unit id="RG984sg" resname="If an account matching your email exists, then an email was just sent that contains a link that you can use to reset your password. This link will expire in %token%.">
        <source>If an account matching your email exists, then an email was just sent that contains a link that you can use to reset your password. This link will expire in %token%.</source>
        <target>Wenn ein Zugang mit Ihrer E-Mail existiert, wurde soeben eine E-Mail dorthin versendet, welche eine Link beinhaltet, mit dem Sie Ihr Passwort zurücksetzen können. Dieser Link wird ablaufen in %token%.</target>
      </trans-unit>
      <trans-unit id="498fEsz" resname="If you don't receive an email please check your spam folder or">
        <source>If you don't receive an email please check your spam folder or</source>
        <target>Wenn Sie keine E-Mail erhalten, überprüfen Sie bitte Ihren Spam Ordner oder/target>
      </trans-unit>
      <trans-unit id="EF54d5d" resname="Try again">
        <source>Try again</source>
        <target>Versuchen Sie es erneut</target>
      </trans-unit>

@cavasinf
Copy link
Collaborator Author

cavasinf commented Jan 6, 2023 •
edited
Loading

  • 1. Can you link the docs page from the docs index
  • 2. _layout.html.twig does not match any filename convention?
  • 3. check_email.html.twig should be check-email.html.twig becuase all files included directly by the controller should be x-y.html.twig - correct?
  • 4. Add a composer suggest for the package
  • 5. Add german translations
  1. I see _layout.html.twig as a "private" tamplate, others template extends from that because security.html.twig does not implement app.flashes but error (which is not an array).
    AND removes login_social_auth blocks
    (See Extra part in DOC)
    Devs should not directly extends that specific file. This is why i named it outside the convention.
  2. Ok but we need to write the bundle contribution rules somewhere 😆

@cavasinf cavasinf added Status: Needs Work Need to be reworked and removed Status: Reviewed Has staff reply/investigation labels Jan 6, 2023
@cavasinf cavasinf marked this pull request as draft January 6, 2023 16:06
@cavasinf cavasinf marked this pull request as ready for review January 16, 2023 08:21
@cavasinf
Copy link
Collaborator Author

Good to go 👍

@cavasinf cavasinf added Status: Reviewed Has staff reply/investigation and removed Status: Needs Work Need to be reworked labels Jan 16, 2023
@cavasinf cavasinf requested a review from kevinpapst January 19, 2023 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevinpapst kevinpapst Awaiting requested review from kevinpapst

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
Feature Feature requested Status: Reviewed Has staff reply/investigation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Though on symfonycasts/reset-password-bundle
2 participants
@cavasinf @kevinpapst