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

Fix UI colors #242

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

XanderD99
Copy link

Thanks a lot for contributing!

Hey, I just found some more UI inconsistencies and tried fixing them all at once.

I removed the "TalkerScreenTheme" class and changed that option with the LogColors property. So now by default all colors for text and background and such just use the theme set by the user.

The LogCards still use the color that is found in the LogColors map. The border of the card uses the color as it is provided and the background is the log color with some opacity added to it. The makes it readable on any theme (at least the colors I tested).

I also changed the default state of the expanded logs to false, if you don't want this I can revert that. I changed that so that the main feed can contain more data and you can then only expand those you want to see more details about. To give errors some more info I set the message to the errorType if no message is given. Errors always show some minor information this way.

If users want to set some custom theme in the log screen they can still wrap it in a Theme widget. Perhaps if needed someone can make a "TalkerTheme" widget that can be used to wrap the talkerScreen?

XanderD99 and others added 8 commits June 17, 2024 10:13
@codecov-commenter
Copy link

codecov-commenter commented Jul 24, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.78%. Comparing base (32fe34e) to head (acfdb3f).
Report is 6 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #242      +/-   ##
==========================================
- Coverage   98.80%   98.78%   -0.02%     
==========================================
  Files          28       28              
  Lines         584      576       -8     
==========================================
- Hits          577      569       -8     
  Misses          7        7              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Frezyx
Copy link
Owner

Frezyx commented Aug 14, 2024

Hello @XanderD99 !
I've already looked at how the updated log screen looks like

For my opinion it's looks very nice. But it's huge breaking change for develeopers who already used talker_flutter to create self logging packages or logs screen customization.

And now I'm thinking about how we can support old TalkerScreenTheme with Deprecated annotation and new UI

@XanderD99
Copy link
Author

I think the best way to have that backwards compability is to keep the TalkerScreenTheme (bring it back in this case) but indeed mark it depricated.
Then when someone uses the TalkerScreenTheme in there project just wrap the whole TalkerScreen in a Theme that uses the colors set in TalkerScreenTheme.

That way the TalkerScreenTheme is removed from al the UI itself but is still available to the other developers until it is completely removed after some time.

@Hirakota
Copy link

It would be batter to change typedef LogColors from Map<TalkerLogType, Color> to Map<String, Color> it's will allow to create custom log types with custom colors

@XanderD99
Copy link
Author

XanderD99 commented Aug 22, 2024

hey @Frezyx I updated the code and added TalkerTheme back to TalkerScreen. I marked is as deprecated however, but when it is set it uses logColors from the talkerTheme and it wraps the TalkerView with a theme that should set background and card colors accordingly.

I haven't fully tested this yet so this should be verified.

The failed build is because of the marked deprecated item

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.

5 participants