search

andyduke / styled_text_package

Text widget with formatted text using tags. Makes it easier to use formatted text in multilingual applications.
https://pub.dev/packages/styled_text
BSD 3-Clause "New" or "Revised" License
75 stars 48 forks source link

Organizing the pull request that allows devs to make superscripts and subscripts easier to build #72

Closed ThomasGysemans closed 1 year ago

ThomasGysemans commented 1 year ago

The builder function of StyledTextWidgetBuilderTag now takes 3 arguments : the context, the attributes (like before) and the textContent of the tag. I added to the examples how to create custom <sup> and <sub> tags.

I also fixed the use of a deprecated method: MediaQuery.boldTextOverride(). It's now MediaQuery.boldTextOf().

I tried to make this pull request as clear as possible without any unnecessary files.

andyduke commented 1 year ago

I tried to make this pull request as clear as possible without any unnecessary files.

Good job. Can you still leave the formatting as in the original? Now it's a little hard to see the changes.

ThomasGysemans commented 1 year ago

Honestly I don't know how to edit a pull request I already made, and the new formatting doesn't change things that much. Sometimes it's useful, but I agree it's annoying when dealing with multiple method calls on a single line, or with ternary operators.

Will it be ok anyway?

andyduke commented 1 year ago

Honestly I don't know how to edit a pull request I already made, and the new formatting doesn't change things that much. Sometimes it's useful, but I agree it's annoying when dealing with multiple method calls on a single line, or with ternary operators.

You can just commit to your branch and the pull request will be updated.

Will it be ok anyway?

In the current state, multiple formatting changes make it very difficult to understand what your code improvement is.

ThomasGysemans commented 1 year ago

You can just commit to your branch and the pull request will be updated

All right, I'll remove the changes due to the formatting and only keep my new code in the .dart files, this afternoon.

In the meantime, you should take a look at my pull request of xmlstream : https://github.com/jorishermans/xmlstream/pull/15 which could help you improve your package. I hope it will get merged, but for this PR too the formatting might cause problems (which I'll fix too).

andyduke commented 1 year ago

In the meantime, you should take a look at my pull request of xmlstream : jorishermans/xmlstream#15 which could help you improve your package. I hope it will get merged, but for this PR too the formatting might cause problems (which I'll fix too).

Nice improvement, but again very difficult to understand what has changed due to noisy reformatting :(

ThomasGysemans commented 1 year ago

I edited both packages so that it's clearer. I hope my changes will help you improve your package.

If you want to take a look at the changes made in xmlstream : https://github.com/jorishermans/xmlstream/pull/15 (which was surprisingly easy).

andyduke commented 1 year ago

@ThomasGysemans Good job.

andyduke commented 1 year ago

@ThomasGysemans Congratulations, your code improvements are online now.