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

Set align to center for titles in the sidebar #180

Closed
wants to merge 6 commits into from
Closed

Set align to center for titles in the sidebar #180

wants to merge 6 commits into from

Conversation

DefaultX-od
Copy link
Contributor

@DefaultX-od DefaultX-od commented Sep 4, 2022

While I was translating software app to Russian, I found out how bizarre, sidebar looks like:

Снимок экрана от 2022-09-07 16-48-37

It looks WAY too big, if you ask me... And I've decided to add new line symbol between words: "Мои" and "Приложения". Here is the result:

Снимок экрана от 2022-09-07 16-49-06

As you can see, text looks strange like that, and I've decided to set text align to center, for every title in the sidebar, even through only one title was affected, because I thought, what if some other language will have way too big title translation to fit in one line, but for different category? And here is the result:

Снимок экрана от 2022-09-07 16-49-41

Looks way more polished. What do you guys think?

Fixes #181

@github-actions
Copy link

github-actions bot commented Sep 4, 2022

Hey! DefaultX-od has not signed the Canonical CLA which is required to get this contribution merged on this project.

Please head over to https://ubuntu.com/legal/contributors to read more about it.

@Jupi007
Copy link
Member

Jupi007 commented Sep 4, 2022

Hi @DefaultX-od,
thanks for your PR, it is much appreciated!

But you need to create a linked issue where we can discuss of the problem before working on a PR :)
You also need to sign the Canonical CLA. If you agree, you have to use the exactly same email address used to push your changes to Github.

I'm currently working on a new version of this navigation rail in yaru_widgets (related issue: ubuntu/yaru.dart#175)
I agree it looks better with a center aligned text.
But imo, it is something which need to be fixed with a generic widget, which define itself text-align: center to its children with a TextTheme.
Like this, we would avoid code repetition.

@Feichtmeier
Copy link
Member

Feichtmeier commented Sep 4, 2022

Thanks @DefaultX-od for the idea
Let's to this in yaru widgets as @Jupi007 suggested. (here is the issue ubuntu/yaru.dart#175)

The navigationrail also has more problems, which is why @Jupi007 is working on a replacement
Could you use this PR-header to create a new issue? your pictures describe very precisely the problem of the current navigation for long text. :)

Thank you for wanting to add russian translations - please do this in another PR if you have time
(And as jupi said don't forget to link an issue to your PR and also sign the CLA with the exact email your commits are made here.) Thanks in advance

@DefaultX-od
Copy link
Contributor Author

изображение

Is this how I should fill in this field?

@DefaultX-od
Copy link
Contributor Author

I've signed CLA just now

@DefaultX-od
Copy link
Contributor Author

Hi @DefaultX-od, thanks for your PR, it is much appreciated!

But you need to create a linked issue where we can discuss of the problem before working on a PR :) You also need to sign the Canonical CLA. If you agree, you have to use the exactly same email address used to push your changes to Github.

I'm currently working on a new version of this navigation rail in yaru_widgets (related issue: ubuntu/yaru_widgets.dart#175) I agree it looks better with a center aligned text. But imo, it is something which need to be fixed with a generic widget, which define itself text-align: center to its children with a TextTheme. Like this, we would avoid code repetition.

Sure, a general solution to a problem is better.

@Jupi007
Copy link
Member

Jupi007 commented Sep 4, 2022

Sure, a general solution to a problem is better.

Thanks @DefaultX-od :)

@DefaultX-od
Copy link
Contributor Author

Thanks @DefaultX-od for the idea Let's to this in yaru widgets as @Jupi007 suggested. (here is the issue ubuntu/yaru_widgets.dart#175)

The navigationrail also has more problems, which is why @Jupi007 is working on a replacement Could you use this PR-header to create a new issue? your pictures describe very precisely the problem of the current navigation for long text. :)

Thank you for wanting to add russian translations - please do this in another PR if you have time (And as jupi said don't forget to link an issue to your PR and also sign the CLA with the exact email your commits are made here.) Thanks in advance

I'm still working on the translation, testing the app with it to find out how do I feel about it. But sure, when I'm done with it, I'll make sure to create an issue first, and then a separate PR for my translation.

@Feichtmeier
Copy link
Member

@DefaultX-od since we agreed on fixing this in yaru_widgets - would you mind closing this PR?

@DefaultX-od
Copy link
Contributor Author

@DefaultX-od since we agreed on fixing this in yaru_widgets - would you mind closing this PR?

Sure.

@DefaultX-od DefaultX-od closed this Sep 5, 2022
@DefaultX-od DefaultX-od deleted the sidebar-text-align-center branch September 5, 2022 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set align to center for titles in the sidebar
3 participants