8000 Unify fundamental terms throughout the application by mabartos · Pull Request #11304 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

Unify fundamental terms throughout the application - #11304

Merged
abstractj merged 1 commit into
keycloak:mainfrom
mabartos:KC-11301
Apr 29, 2022
Merged

Unify fundamental terms throughout the application#11304
abstractj merged 1 commit into
keycloak:mainfrom
mabartos:KC-11301

Conversation

@mabartos
@mabartos mabartos commented Apr 14, 2022 •
edited
Loading
Copy link
Copy Markdown
Member

Fixes #11301

@andymunro @tyandor Could you please take a look at this? Thank you.

@tyandor
tyandor commented Apr 14, 2022
Copy link
Copy Markdown
Contributor

Thanks for the heads up @mabartos! Changes we made here were requested by UX but if that creates inconsistency definitely not a problem to change back.

tyandor
tyandor approved these changes Apr 14, 2022
View reviewed changes
@tyandor tyandor left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me 👍

@andymunro
andymunro commented Apr 14, 2022
Copy link
Copy Markdown
Contributor

Generally our documentation always use log in rather than sign in. I think it is a good topic to suggest for the whole documentation set, but not for one PR.

@mabartos
mabartos commented Apr 19, 2022
Copy link
Copy Markdown
Member Author

@andymunro I'm not sure about this. In PR #10748, there were some changes related to those terms and I don't see any benefit by changing it there, rather the opposite. The account console was using the Sign in term before those changes[1] and it was consistent with the login screens. The documentation is more separated than the login and account screens together. I think this PR is relevant.

[1] https://github.com/keycloak/keycloak/pull/10748/files#diff-c9960d63791e169b1bd22eaa9d37a256f2e76c3738a9aa35365f1f62e9ec0ccaL98-L104

@andymunro
andymunro commented Apr 19, 2022
Copy link
Copy Markdown
Contributor

Thanks @mabartos. Okay, I see either way, there is some inconsistency with the UI or the documentation. If so, I agree with your suggested changes.

8000
@ahus1 ahus1 added area/account/ui Account Console — React UI for profile, sessions, security settings kind/bug Categorizes a PR related to a bug labels Apr 20, 2022
@abstractj
abstractj requested a review from ssilvert April 26, 2022 12:40
@abstractj abstractj added area/admin/ui Admin Console — React UI for server administration and removed area/admin/ui Admin Console — React UI for server administration labels Apr 26, 2022
@abstractj
abstractj commented Apr 26, 2022
Copy link
Copy Markdown
Contributor

Adding @ssilvert here considering the impact in the UI

@abstractj
abstractj merged commit d931156 into keycloak:main Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@abstractj abstractj abstractj approved these changes
@ssilvert ssilvert Awaiting requested review from ssilvert
@agagancarczyk agagancarczyk Awaiting requested review from agagancarczyk
@edewit edewit Awaiting requested review from edewit
+3 more reviewers
@jonkoops jonkoops jonkoops approved these changes
@tyandor tyandor tyandor approved these changes
@andymunro andymunro andymunro approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/account/ui Account Console — React UI for profile, sessions, security settings kind/bug Categorizes a PR related to a bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Unify fundamental terms throughout the application

6 participants

@mabartos @tyandor @andymunro @abstractj @jonkoops @ahus1
0