Skip to content

Conversation

@hannesa2
Copy link
Collaborator

It's a follow up of #341 with screenshot compare
@daniel-marcus-kroger

@hannesa2
Copy link
Collaborator Author

Looks good


StartTest_smokeTestStart-1-LineChartActivity-Basic-1SampleClick.pngscreenshot

@hannesa2 hannesa2 changed the title Enable vertical shading Enable vertical limit range shading Apr 13, 2025
@hannesa2 hannesa2 force-pushed the limit-range branch 4 times, most recently from 5778cf8 to 7c3981c Compare April 14, 2025 05:52
@daniel-marcus-kroger
Copy link

daniel-marcus-kroger commented Apr 14, 2025

Thank you very much, it's good to see how the screenshot compare testing works!

More than happy to have this version merged!

@hannesa2 hannesa2 force-pushed the limit-range branch 9 times, most recently from 408711a to 559e3c5 Compare April 15, 2025 10:55
@daniel-marcus-kroger
Copy link

@hannesa2 , it looks like this PR has the renderer code converted to Kotlin (looks nice!) but is resulting in some failures on API level 9, if I am reading the completely black rectangle correctly. Is there anything you'd like me to do to help shore this up?

@hannesa2 hannesa2 force-pushed the limit-range branch 2 times, most recently from dae3fcf to 8ad7321 Compare April 18, 2025 06:11
@hannesa2
Copy link
Collaborator Author

Unfortunately I didn't merged as it was working properly and I merged some Kotlin conversions in advance. This was not good. But it's now like it is.

Black screenshot means that there is a crash.

I fixed the crash now

@hannesa2 hannesa2 force-pushed the limit-range branch 3 times, most recently from fc69e03 to 80ef8b7 Compare April 18, 2025 07:24
@hannesa2
Copy link
Collaborator Author

Hmm, but I see it

  • looses the axis labels
  • Middle range is confusing and overlapping

Any help is welcome @daniel-marcus-kroger

@hannesa2 hannesa2 merged commit 0dbfcf5 into master Apr 18, 2025
2 checks passed
@hannesa2 hannesa2 deleted the limit-range branch April 18, 2025 16:43
@daniel-marcus-kroger
Copy link

@hannesa2 My apologies for the delayed reply, I was out of the office for a couple of days. It looks like you got everything working. I will make sure to use the new Kotlin versions if I implement this for vertical region shading, and that will hopefully be much easier to merge!

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.

3 participants