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

Answer:55 #1037

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

Answer:55 #1037

wants to merge 2 commits into from

Conversation

tsironis13
Copy link
Contributor

No description provided.

@github-actions github-actions bot added 55 answer answer contributor highly contributing to the project to be reviewed PR requests a review labels Jul 6, 2024
@tsironis13
Copy link
Contributor Author

@tomalaforge hello! This is my proposed solution...shall I create a link to this page for the UI?

tomalaforge
tomalaforge reviewed Jul 8, 2024
View reviewed changes
apps/angular/55-back-button-navigation/src/app/dialog.service.ts
@Injectable({
providedIn: 'root',
})
export class DialogService {
Copy link
Owner

Choose a reason for hiding this comment

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

Nice service, a bit out of the challenge, but great generic service for people to add to their projects 👍

tomalaforge
tomalaforge reviewed Jul 8, 2024
View reviewed changes
apps/angular/55-back-button-navigation/src/app/simple-action/simple-action.component.ts
selector: 'app-simple-action',
templateUrl: './simple-action.component.html',
})
export class SimpleActionComponent {
readonly #dialog = inject(MatDialog);
export class SimpleActionComponent implements CanComponentDeactivate {
Copy link
Owner

Choose a reason for hiding this comment

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

👍

tomalaforge
tomalaforge reviewed Jul 8, 2024
View reviewed changes
apps/angular/55-back-button-navigation/src/app/can-deactivate.guard.ts
component: CanComponentDeactivate,
) => {
return component.canDeactivate ? component.canDeactivate() : true;
};
Copy link
Owner

Choose a reason for hiding this comment

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

👍

@tomalaforge
Copy link
Owner

I was surprised about the number of change, but you implemented a generic way of displaying dialog. Not really the challenge, but great way to show how to implement generic dialog inside an entreprise application. 🔥
People can get inspired by your implementation. Thanks for sharing

@tomalaforge tomalaforge added answer author answer of author and removed answer answer to be reviewed PR requests a review labels Jul 8, 2024
@tomalaforge
Copy link
Owner

@tomalaforge hello! This is my proposed solution...shall I create a link to this page for the UI?

What do you mean?
If it's about the website, the label answer author and the number are enough to link your PR to your challenge 👍

@tsironis13
Copy link
Contributor Author

@tomalaforge hello! This is my proposed solution...shall I create a link to this page for the UI?

What do you mean? If it's about the website, the label answer author and the number are enough to link your PR to your challenge 👍

xm yes it works out of the box...thanks

@tsironis13
Copy link
Contributor Author

I was surprised about the number of change, but you implemented a generic way of displaying dialog. Not really the challenge, but great way to show how to implement generic dialog inside an entreprise application. 🔥 People can get inspired by your implementation. Thanks for sharing

well thanks first of all!...actually the idea and the purpose of this challenge is to combine both Angular features(like 'CanDeactivate' route guards) with Typescript and generic design patterns to provide a scalable solution. It can moved also to Typescript region/category, if you think it is better suited there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomalaforge tomalaforge tomalaforge left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
55 answer author answer of author contributor highly contributing to the project
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tsironis13 @tomalaforge