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

[FIX] No pattern for user's status text capitalization #9783

Merged
merged 1 commit into from
Mar 26, 2018
Merged

[FIX] No pattern for user's status text capitalization #9783

merged 1 commit into from
Mar 26, 2018

Conversation

lunaticmonk
Copy link
Contributor

@lunaticmonk lunaticmonk commented Feb 20, 2018 •
edited by rodrigok
Loading

@RocketChat/core

Closes #9547

screen shot 2018-02-20 at 5 12 18 pm

@lunaticmonk
Copy link
Contributor Author

@rafaelks @JSzaszvari Please review.

@karlprieb
Copy link
Contributor

@sumedh123 can't we do that using only CSS?

@engelgabriel do we want that? I really don't see the problem with lowercase on that case.

@engelgabriel
Copy link
Member

@karlprieb yes, IF we do it, should be ONLY css, but I don't think we should force this here, we should just fix the translation files and make sure we are using the same string in all places.

@lunaticmonk
Copy link
Contributor Author

@karlprieb @engelgabriel Should I try this with CSS only or do we follow any other approach?

@graywolf336
Copy link
Contributor

@sumedh123 As Gabriel said, change the i18n setup so that it's all the same and using the same string in every place.

@karlprieb
Copy link
Contributor

But we really want that? I think it's better to be lowercase. What you guys think? @RocketChat/core

@rafaelks
Copy link
Contributor

@karlprieb I prefer all status in lowercase (in all places).

@geekgonecrazy
Copy link
Contributor

geekgonecrazy commented Feb 22, 2018 •
edited
Loading

Yeah I think visually it looks good lowercase

@lunaticmonk lunaticmonk force-pushed the capitalize-status branch 2 times, most recently from 7ab51e6 to 737f7d7 Compare February 22, 2018 21:27
@lunaticmonk
Copy link
Contributor Author

@karlprieb @geekgonecrazy Please review.

@lunaticmonk lunaticmonk changed the title Capitalize the first letter of user status in the header below the us… Lowercase the whole user status in the header below the us… Feb 25, 2018
karlprieb
karlprieb suggested changes Feb 26, 2018
View reviewed changes
Copy link
Contributor

@karlprieb karlprieb left a comment

Choose a reason for hiding this comment

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

We already have lowecase keys for all status like "away" instead of "Away" we should update the template that use "Away" for "away" for example.

@lunaticmonk
Copy link
Contributor Author

@karlprieb Done 👍 Please review again. Thanks.

@rodrigok rodrigok changed the title Lowercase the whole user status in the header below the us… [FIX] No pattern for user's status text capitalization Mar 26, 2018
@rodrigok rodrigok merged commit 70552f8 into RocketChat:develop Mar 26, 2018
@rodrigok rodrigok added this to the 0.63.0 milestone Mar 26, 2018
@rodrigok rodrigok mentioned this pull request Apr 4, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karlprieb karlprieb karlprieb approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.63.0
Development

Successfully merging this pull request may close these issues.

Statuses beneath username on a Direct Message begins with a lower case letter
7 participants
@lunaticmonk @karlprieb @engelgabriel @graywolf336 @rafaelks @geekgonecrazy @rodrigok