Closed BachToTheFuture closed 5 years ago
@pulkomandy @mmuman Could you review this GCI task?
I just found a bug. See below. See if you can track it down.
@owenca Negative time... interesting ๐ฎ Maybe we're traveling backwards in time?
@owenca Hmm. What were the starting and ending times for Sleep?
@bach5000 Starting: 23:00, Ending: 8:00 next day
@owenca Hmm I have no problems. Shows Starts in 10hr, 2 min, 56 sec
.
Strange.
With your new changes, it has the same bug.
By the way, this is in VirtualBox running 32-bit Haiku.
@owenca Might have to do with arch differences... I'm running on x64. Maybe that's why. I'll test it on 32 bit and see if I can recreate the bug.
I suspected that you were running on 64-bit and that it was an integer overflow bug. :)
@owenca Perhaps :)
@owenca I think I fixed it. I wasn't suppose to use the minus operator.
Is this ready for merge or is there still a pending fix or two needed?
@bach5000 Can you address the coding style issue on line 221? See my comments above.
@bach5000 Thanks! @pulkomandy @mmuman Can you have a look at the PR before it's merged?
@pulkomandy Flipped the event name and time position. This is what it looks like now:
I'm still confused about BString.ReplaceAll
, though. Do you want me to use it to replace %remaining
in Now, %remaining left
with the time?
@bach5000 Do something like this:
BString timeLeft("Now, %remaining% left");
timeLeft.ReplaceAll("%remaining%", remaining);
timePeriod << timeLeft;
And similarly for other strings.
@owenca That makes more sense now. Thanks for clarifying ๐
@owenca @pulkomandy Patched the code according to the suggestions :)
@pulkomandy @owenca @scottmc Done!
@janus2 @pulkomandy Is there a function to get system color values? Currently all I could think of is simply put everything (event name and time) on B_LIGHTEN_1_TINT or set just the name's tint_color value to 0.5. This makes the font gray, and gray font is visible on both white and black backgrounds... but there's a problem when the background is gray :(
Is this ready for merge?
@bach5000 you can check the sum of the components like https://github.com/haiku/haiku/blob/abb59d7351c7ddb50c63c40430a82d94fa61917a/src/preferences/appearance/ColorWhichItem.cpp#L69
or you can use the function Brightness() present in the color struct
You get the system colors using ui_color (or even better, use SetViewUIColor functions so it live updates when changed in preferences).
But here you need ui_color and then decide if it's "dark" or "light" to derive another color from it.
@bach5000 You can reference the code here to see what @pulkomandy is talking about.
I think we should have a function like tint_color that keeps into account the luminance of the color. In the code there are different checks that can result in an inconsistent gui. This is more visible with replicants on the desktop, with the same background color some are white some black.
In this specific case, maybe using the default color and an alpha of 50% or so would work, rather than tinting?
@owenca I'm really curious... how are you finding all these perfect code snippets? Anyways, using @owenca's useful code snippet and @pulkomandy's alpha suggestion, perhaps I could do something like this?
rgb_color color;
color = ui_color(B_LIST_ITEM_TEXT_COLOR);
color.alpha = 0.5;
view->SetHighColor(color);
@pulkomandy @owenca @janus2 Okay... seems like using alpha didn't work, so I decided to try something else. Were these what you guys wanted?
You scheduled no time to sleep... Why are there the two black grey boxes next to November 2018 on the second image?
@scottmc Those gray boxes are suppose to be navigation buttons for changing the month. I forgot to add the symbols on the buttons when I took the pictures.
@bach5000 in my opinion for the event name you should use directly the B_LIST_ITEM_TEXT_COLOR/B_LIST_SELECTED_ITEM_TEXT_COLOR without any tint (in the screenshots "Sleep", "Testing", "Dinner"), and for the time interval ("12 AM - 12 AM") a lighter color if the B_LIST_ITEM_TEXT_COLOR/B_LIST_SELECTED_ITEM_TEXT_COLOR is dark and a darker color if is bright. A cut and paste of the snippets in previous comments should work.
@janus2 So when event text (B_LIST_ITEM_TEXT_COLOR) is dark, then the time interval text should be lighter, and if event text is light, then the time interval should be lighter? Okay. I think I understand what you guys expected... I misunderstood in the beginning.
@janus2 Is this what you expected?
I cannot test the code now, but the logic looks correct to me.
@owenca I'm really curious... how are you finding all these perfect code snippets?
@bach5000 They are from my past commits. :) I learned from @humdingerb that you canโt rely on the default color theme and just assume your design would work for every preference setting. That code snippet was the result of hours of digging and experimenting with all sorts of color combinations against various preference settings.
@owenca Seems like you gained a lot of experience from GCI ๐ Thanks for helping, by the way :)
Just doing my job. This is what mentoring is about, right?
This ready for merge now?
Fixes #47 New features + changes:
Now, [time] left
Finished!
Starts in [time]
Day view
toDay
,Week view
toWeek
... I don't know if this is needed, though. I removed it because I thought it was too repetitive once I add inAgenda view