-
-
Notifications
You must be signed in to change notification settings - Fork 40
Fix/align focus text #138
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
Fix/align focus text #138
Conversation
|
Why do you keep spamming PRs without creating issues? This is basic contribution etiquette. And your change doesn't even fix anything, it actually breaks the existing animation. Did you even run the app after this change? Do it and see the animation and comment here if you still think this PR can be merged. |
|
Take it easy my friend, all 3 PR's was already created before you told me to open issue first, so I did not know. This PR is a fix for an opened issues, if you like my solution merge it, if not decline it I am just trying to help and fix some of those small bugs |
Not really, the animation is broken BeforeScreen_recording_20251110_140859.mp4AfterScreen_recording_20251110_141002.mp4 |
I'm really sorry if I sounded rude. I received the mails for your PRs with quite a bit of delay, so I assumed you had read my earlier comments. Anyway, you should discuss whatever implementation you are going to put in your PR with the maintainer at the very least, especially for code that affects the design like this one. Reviewing surprise PRs can take a bit of time off my regular schedule. And even if I ignore all of that, you didn't even perform the due diligence of at least running the app to test your changes before making a PR. And you posted screenshots to show that the animation isn't broken, which is a bit weird lol I assume it was a mistake/misunderstanding from your side, and so I don't really mind these PRs at all, it's fine and I would suggest you to keep contributing to this project! It is difficult to determine tone from text, so I might have sounded rude in my previous comment but I didn't really intend to. I'm sorry for that again. |
|
No worries The screenshots was to show that the centering was fixed, you said the change did not fix anything :) Anyways, I will the animation and update the PR. Sorry for all this |
|
I was in the middle of a class when I responded initially and I didn't pay attention to what I wrote, so I accidentally wrote that "doesn't fix anything" bit 😅 I'm sorry if I sounded rude lol |
|
As expected, in Option 1: is to dig deeper into What do you think ? |
d0bb70c to
3b22223
Compare
|
@nsh07 can you take a look at this please |
|
This works, thank you for the fix! Also, I would like to apologize once again for the way I talked to you before. I hope you didn't mind that :( |
|
No worries my friend |


Based on #123
Apparently
TopAppBarhave slight padding on the right.Alignment.Centeris enough to center the text, no need forfillMaxWidth()