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

home: Center align the "try another account" message in loading placeholder #1326

Merged
merged 1 commit into from
Feb 19, 2025

Conversation

KYash03
Copy link
Contributor

@KYash03 KYash03 commented Feb 5, 2025

Resolves #1246.

Screenshot

Screenshot

Copy link
Collaborator

@chrisbobbe chrisbobbe left a comment

Choose a reason for hiding this comment

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

Thanks! Small comments below.

@KYash03 KYash03 closed this Feb 5, 2025
@KYash03 KYash03 force-pushed the center-align-tryAnotherAccountMessage branch from 402d967 to 7659479 Compare February 5, 2025 22:59
@KYash03 KYash03 reopened this Feb 5, 2025
@KYash03
Copy link
Contributor Author

KYash03 commented Feb 5, 2025

Hi @chrisbobbe, I've made the changes. Please review. Apologies for the earlier confusion with closing and reopening the PR!

@chrisbobbe
Copy link
Collaborator

Bump on #1326 (comment) ; please pay attention to spacing/linebreaks/indentation.

@KYash03 KYash03 force-pushed the center-align-tryAnotherAccountMessage branch from f217e16 to dc49661 Compare February 6, 2025 03:13
Comment on lines 215 to 217
Text(textAlign: TextAlign.center,
zulipLocalizations.tryAnotherAccountMessage(account.realmUrl.toString()),
),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this what you're expecting, @chrisbobbe?

Copy link
Collaborator

Choose a reason for hiding this comment

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

                    Text(textAlign: TextAlign.center,
                      zulipLocalizations.tryAnotherAccountMessage(account.realmUrl.toString())),

as I said in #1326 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed now. Sorry for the confusion earlier, GitHub's formatting made it hard to see exactly what was expected.

@KYash03 KYash03 force-pushed the center-align-tryAnotherAccountMessage branch from dc49661 to 21e496b Compare February 7, 2025 01:56
@PIG208 PIG208 self-assigned this Feb 12, 2025
@PIG208 PIG208 added the maintainer review PR ready for review by Zulip maintainers label Feb 12, 2025
@PIG208
Copy link
Member

PIG208 commented Feb 12, 2025

Thanks @KYash03 for working on this and Chris for the previous reivews! This looks good to me.

@PIG208 PIG208 added integration review Added by maintainers when PR may be ready for integration and removed maintainer review PR ready for review by Zulip maintainers labels Feb 12, 2025
@PIG208 PIG208 assigned gnprice and unassigned PIG208 Feb 12, 2025
@chrisbobbe chrisbobbe requested a review from gnprice February 12, 2025 21:42
@gnprice
Copy link
Member

gnprice commented Feb 19, 2025

Thanks for taking care of this!

Looks good; merging, with one small fix:

$ git range-diff origin pr/1326 @
1:  21e496b1d ! 1:  4f438d341 home: Center align the "try another account" message in loading placeholder
    @@ Metadata
      ## Commit message ##
         home: Center align the "try another account" message in loading placeholder
     
    -    Fixes #1246
    +    Fixes #1246.
     

(Note the punctuation in Chris's comment #1326 (comment) above.)

@gnprice gnprice force-pushed the center-align-tryAnotherAccountMessage branch from 21e496b to 4f438d3 Compare February 19, 2025 04:40
@gnprice gnprice merged commit 4f438d3 into zulip:main Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration review Added by maintainers when PR may be ready for integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tryAnotherAccountMessage stops being center-aligned when text is long enough to wrap
4 participants