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

Feature/authentication frontend #64

Merged
merged 54 commits into from
Jun 14, 2024

Conversation

XanderVertegaal
Copy link
Contributor

@XanderVertegaal XanderVertegaal commented May 4, 2024

This PR addresses a number of points listed under 'Frontend' in #38. It also introduces (NgBootstrap) toasts for app-wide notifications.

There are a few issues that could still be implemented. This PR is getting quite big, however, so I wanted to have this reviewed first before I introduce more moving parts / complexity.

Open issues

validators: identicalPasswordsValidator<keyof RegisterForm>('password1', 'password2')
});

public usernameErrors$ = controlErrorMessages$(this.form, 'username');
Copy link
Contributor Author

@XanderVertegaal XanderVertegaal May 4, 2024

Choose a reason for hiding this comment

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

Instead of creating an stream of error messages here and using them with | async in the template, we could also create a pipe that extracts them from a control in the template. Something like:

<p *ngFor="let error of form.controls.username | controlErrors ">{{ error }}</p>

Base automatically changed from feature/authentication-basics to develop May 13, 2024 10:29
Copy link
Contributor

@lukavdplas lukavdplas left a comment

Choose a reason for hiding this comment

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

Kudos for how much you've done here!

Comment on lines 17 to 25
medieval_title = models.CharField(
max_length=255, blank=True, help_text="The original title of the work, if known"
)

medieval_author = models.CharField(
max_length=255,
blank=True,
help_text="The name of the original author of the work, if known",
)
Copy link
Contributor

Choose a reason for hiding this comment

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

original_title / original_author sounds more appropriate to me, but I'll leave it to your judgement.

backend/user/views.py Show resolved Hide resolved
frontend/src/app/services/auth.service.ts Outdated Show resolved Hide resolved
frontend/src/app/app.component.ts Outdated Show resolved Hide resolved
frontend/src/app/user/login/login.component.html Outdated Show resolved Hide resolved
frontend/src/app/user/login/login.component.ts Outdated Show resolved Hide resolved
@XanderVertegaal XanderVertegaal marked this pull request as ready for review May 15, 2024 11:52
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@XanderVertegaal XanderVertegaal merged commit eefd79e into develop Jun 14, 2024
2 checks passed
@XanderVertegaal XanderVertegaal deleted the feature/authentication-frontend branch June 14, 2024 07:06
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.

None yet

2 participants