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

feat(incorrect-information-button): Information button about how incorrect data can be changed #382

Merged
merged 59 commits into from
Dec 4, 2019
Merged

feat(incorrect-information-button): Information button about how incorrect data can be changed #382

merged 59 commits into from
Dec 4, 2019

Conversation

Zeroks77
Copy link
Collaborator

So here we go again :D

  • created button with dialog explaining that the user can't change data via Phonebook
  • Dialog changes depends on, wether you are on your or on a colleagues profile
  • add translationen for the dialog

ISSUES CLOSED: #174

@Zeroks77 Zeroks77 added the feature An enhancement or a new functionality feature. label Oct 21, 2019
@Zeroks77 Zeroks77 changed the title feat: add incorrect data button, add translation feat: add incorrect data button Oct 21, 2019
mschwrdtnr
mschwrdtnr reviewed Oct 21, 2019
View reviewed changes
Copy link
Member

@mschwrdtnr mschwrdtnr left a comment

Choose a reason for hiding this comment

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

I will review the other stuff once the preview is working

Phonebook.Frontend/.gitignore Outdated Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/components/user/user-detail/user-detail.component.html Outdated Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/components/user/user-detail/user-detail.component.html Outdated Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/components/user/user-detail/user-detail.component.scss Outdated Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/components/user/user-detail/user-detail.component.scss Outdated Show resolved Hide resolved
DanielHabenicht
DanielHabenicht reviewed Oct 21, 2019
View reviewed changes
Phonebook.Frontend/src/app/shared/components/user/user-detail/user-detail.component.ts Outdated Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/components/user/user-detail/user-detail.component.ts Outdated Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/components/user/user-detail/user-detail.component.ts Outdated Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/dialogs/dialogs.module.ts Outdated Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/dialogs/user-information/user-information.component.scss Outdated Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/dialogs/user-information/user-information.component.scss Outdated Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/dialogs/user-information/user-information.component.spec.ts Outdated Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/dialogs/user-information/user-information.component.ts Outdated Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/dialogs/user-information/user-information.component.html Outdated Show resolved Hide resolved
mschwrdtnr
mschwrdtnr requested changes Oct 23, 2019
View reviewed changes
Copy link
Member

@mschwrdtnr mschwrdtnr left a comment

Choose a reason for hiding this comment

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

image
Scroll-Bar in Chrome will be fixed with my changes

Please be more aware of the given functions and styles next time

Phonebook.Frontend/src/app/shared/components/user/user-detail/user-detail.component.html Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/components/user/user-detail/user-detail.component.scss Outdated Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/components/user/user-detail/user-detail.component.scss Outdated Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/components/user/user-detail/user-detail.component.ts Outdated Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/components/user/user-detail/user-detail.component.ts Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/dialogs/user-information/user-information.component.ts Outdated Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/components/user/user-detail/user-detail.component.ts Outdated Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/dialogs/user-information/user-information.component.html Outdated Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/dialogs/user-information/user-information.component.html Outdated Show resolved Hide resolved
Phonebook.Frontend/src/app/shared/dialogs/user-information/user-information.component.scss Outdated Show resolved Hide resolved
@DanielHabenicht
Copy link
Collaborator

DanielHabenicht commented Oct 23, 2019 •
edited
Loading

image
Scroll-Bar in Chrome will be fixed with my changes

Please be more aware of the given functions and styles next time

As I wrote all other Dialogs are aligned left. Why is this different?

@mschwrdtnr
Copy link
Member

image
Scroll-Bar in Chrome will be fixed with my changes
Please be more aware of the given functions and styles next time

As I wrote all other Dialogs are aligned right. Why is this different?

We can commit on right aligned Dialogs. In this case, @Zeroks77 you can also use the provided classes.

@DanielHabenicht
Copy link
Collaborator

Sry to bug you out. But the component files name has to be written in lowercase.

@Zeroks77
Copy link
Collaborator Author

yikes XD it's early in the morning my bad

@Zeroks77
Copy link
Collaborator Author

Zeroks77 commented Oct 24, 2019 •
edited
Loading

@DanielHabenicht why do you want this dialog to be aligned to the right?
I think it would look inappropriate in this case.

image

@mschwrdtnr
Copy link
Member

@DanielHabenicht why do you want this dialog to be aligned to the right?
I think it would look inappropriate in this case.

image

I would also center it. It looks not so good with this alignment.

@DanielHabenicht
Copy link
Collaborator

DanielHabenicht commented Oct 24, 2019 •
edited
Loading

@DanielHabenicht why do you want this dialog to be aligned to the right?
I think it would look inappropriate in this case.
image

I would also center it. It looks not so good with this alignment.

Moving the button right like in other dialogs? I'm still agains the formality of "Dear Stone". I think it useless, it draws my attention but does not add any information + we are already deep into the app. We are not even writing "Dear" in the startup dialog.

@DanielHabenicht
Copy link
Collaborator

Apart from that it looks ready to me.

@Zeroks77
Copy link
Collaborator Author

@DanielHabenicht why does the tests fail ?
image
they work localy fine

@DanielHabenicht
Copy link
Collaborator

DanielHabenicht commented Oct 24, 2019 •
edited by Zeroks77
Loading

@Zeroks77 as marcus already said the chromedriver and chrome version on CI are different. As they work for you can you tell me which chrome version you are using?

@DanielHabenicht
Copy link
Collaborator

have a look at the log output "details" -> "View more details on Azure Pipelines"-> Navigate to red step

@Zeroks77
Copy link
Collaborator Author

@DanielHabenicht I'm using Chrome Version 77.0.3865.120 (official build) (64-Bit)

@mschwrdtnr
Copy link
Member

Apart from that it looks ready to me.

@DanielHabenicht what are we waiting for?

mschwrdtnr
mschwrdtnr previously approved these changes Oct 25, 2019
View reviewed changes
@DanielHabenicht
Copy link
Collaborator

Alignment of the text and the failing builds.

@Zeroks77
Copy link
Collaborator Author

Zeroks77 commented Oct 25, 2019 •
edited
Loading

I dont get it.... why does the build fail in this pr but in every other dont ?

I mean localy everything works fine

@DanielHabenicht
Copy link
Collaborator

@Zeroks77 look at he log! Seems like a component is not found. Check the paths.

@DanielHabenicht
Copy link
Collaborator

Maybe as @paule96 for help :)

@mmsgithub-ci
Copy link
Collaborator

Preview Environment ready at https://pr-382.demo-phonebook.me

DanielHabenicht
DanielHabenicht previously approved these changes Dec 4, 2019
View reviewed changes
mschwrdtnr
mschwrdtnr requested changes Dec 4, 2019
View reviewed changes
Copy link
Member

@mschwrdtnr mschwrdtnr left a comment

Choose a reason for hiding this comment

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

This should be the last :)

...ook.Frontend/src/app/shared/dialogs/user-information/incorrect-user-information.component.ts Outdated Show resolved Hide resolved
...ook.Frontend/src/app/shared/dialogs/user-information/incorrect-user-information.component.ts Outdated
meaning: 'MailToMateBody',
description: 'Send a mail to your mate if there is something wrong on the profile ',
id: 'User-InformationDialogBody',
value: ', \n while browsing your profile I noticed that something is not right: \n\n\n Please contact the HR Department to fix it. This is the Phonebook Link: '
Copy link
Member

Choose a reason for hiding this comment

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

here are two wrong spaces at the beginning:
image

Phonebook.Frontend/src/i18n/messages.de.xlf Outdated Show resolved Hide resolved
Phonebook.Frontend/src/i18n/messages.de.xlf Outdated Show resolved Hide resolved
...ook.Frontend/src/app/shared/dialogs/user-information/incorrect-user-information.component.ts Outdated Show resolved Hide resolved
@Zeroks77 @mschwrdtnr
Update Phonebook.Frontend/src/app/shared/dialogs/user-information/inc…
c50a588 
…orrect-user-information.component.ts

Co-Authored-By: Max Schwerdtner <[email protected]>
@Zeroks77 Zeroks77 dismissed stale reviews from DanielHabenicht via c50a588 December 4, 2019 13:54
@mmsgithub-ci
Copy link
Collaborator

Preview Environment ready at https://pr-382.demo-phonebook.me

DanielHabenicht
DanielHabenicht requested changes Dec 4, 2019
View reviewed changes
Copy link
Collaborator

@DanielHabenicht DanielHabenicht left a comment

Choose a reason for hiding this comment

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

Sry, hast du dir selbst eingebrockt.

Phonebook.Frontend/src/i18n/messages.de.xlf Outdated Show resolved Hide resolved
@Zeroks77
Copy link
Collaborator Author

Zeroks77 commented Dec 4, 2019 •
edited
Loading

Noch 14 Kommentare :D

Ich erwarte noch was

@mmsgithub-ci
Copy link
Collaborator

Preview Environment ready at https://pr-382.demo-phonebook.me

@DanielHabenicht
Copy link
Collaborator

Ne mindestens 18 sonst knacken wir gar nicht die 200

mschwrdtnr
mschwrdtnr approved these changes Dec 4, 2019
View reviewed changes
Copy link
Member

@mschwrdtnr mschwrdtnr left a comment

Choose a reason for hiding this comment

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

🎉 🎉 🎉 🎉

@mschwrdtnr mschwrdtnr merged commit fcb5125 into master Dec 4, 2019
@mmsgithub-ci
Copy link
Collaborator

🎉 This PR is included in version 1.36.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@Zeroks77 Zeroks77 deleted the feat/incorrect_button branch March 24, 2020 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mschwrdtnr mschwrdtnr mschwrdtnr approved these changes

@DanielHabenicht DanielHabenicht DanielHabenicht approved these changes

@paule96 paule96 Awaiting requested review from paule96

@marcuskrahl marcuskrahl Awaiting requested review from marcuskrahl

Assignees
No one assigned
Labels
feature An enhancement or a new functionality feature. released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Information is incorrect - Button
5 participants
@Zeroks77 @DanielHabenicht @mschwrdtnr @mmsgithub-ci @VanLampe