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

[FeedbackNeeded] test color report (drTest/Calypso) #5264

Merged
merged 15 commits into from
Feb 5, 2020

Conversation

hogoww
Copy link
Contributor

@hogoww hogoww commented Nov 29, 2019

Started to change colors used by DrTest and Calypso, to be less flashy.
Not ready, want to change some more stuff with Julien a bit later today.

commit number 3 is lacking a part of its message: 'and tweeked color to be less flashy'
overs are explicit

@hogoww hogoww changed the title [Do Not Integrate] Dr test color [Do Not Integrate] test color report (drTest/Calypso) Nov 29, 2019
@hogoww
Copy link
Contributor Author

hogoww commented Nov 29, 2019

SimpleButtonMorph >> doButtonAction was as far as I went to find Calypso's colors.
But I feel they are fine in comparison, being so small and without text, so i don't know if i'll investigate further.

@hogoww
Copy link
Contributor Author

hogoww commented Nov 29, 2019

Ok, so now we have default color described in Sunit. gonna try to find "better" colors for tests.

@dionisiydk Didn't find the color for unit tests in Calypso yet. Could you give me an hint as where I should look please?

@hogoww
Copy link
Contributor Author

hogoww commented Nov 29, 2019

Attempted to have better coloring for DrTest/Calypso test. Would love some feedback to be able to know what would be nicer.

Important note: Spec2 doesn't support coloring for buttons label's, so we're stuck with white text in dark mode for now.

@hogoww hogoww changed the title [Do Not Integrate] test color report (drTest/Calypso) [FeedbackNeeded] test color report (drTest/Calypso) Nov 30, 2019
Copy link
Contributor

@juliendelplanque juliendelplanque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems nice, little detail to discuss, see related comments.

src/SUnit-Core/TestResult.class.st Outdated Show resolved Hide resolved
src/SUnit-Core/TestResult.class.st Outdated Show resolved Hide resolved
src/SUnit-Core/TestResult.class.st Outdated Show resolved Hide resolved
@estebanlm
Copy link
Member

  1. I need a screenshot to evaluate the before/after of the changes
  2. I like to remove hardcoded colours
  3. This will be for Pharo9.0 now :)

@estebanlm estebanlm changed the base branch from Pharo8.0 to Pharo9.0 December 20, 2019 13:59
@hogoww
Copy link
Contributor Author

hogoww commented Dec 25, 2019

For the dark theme:
Personnal notes first:

  • I cannot read anything on the current success/failure, error is okay.
  • My screen is probably too bright, but on it some colors are just blinding. Got to hide my drtest under another window 1
  • We added support for default font color, but Spec does not support it yet (soon !). Still, this prevents us to do better, by replacing the font color and have a nice color, with readable text.
    (also, my color vision is not the best..)

success new
success_dark_new
success old
dark_success_old

failure new
Screenshot 2019-12-25 at 17 21 12

failure old
dark_failure_old

error new
error_dark_old
error old
dark_error_old

@hogoww
Copy link
Contributor Author

hogoww commented Dec 25, 2019

light theme
Only failure was unusable.
I'm not sure why text is white and don't remember if I once knew.
This is making it worse though.
iirc the text color follows the theme (white text dark theme / black text white theme...)

Success new
light_success_new
success old
light_success_old

failure new
light_failure_new
failure old
light_failure_old

error new
error_failure_new
error old
light_error_old

@Ducasse
Copy link
Member

Ducasse commented Dec 26, 2019

I usually cannot read comments in the black theme and the red is also far too strong I cannot read text in it.

@Ducasse
Copy link
Member

Ducasse commented Dec 26, 2019

So take care of it is really important.

@hogoww
Copy link
Contributor Author

hogoww commented Dec 26, 2019

@Ducasse "I usually cannot read comments in the black" Alright, but this is unrelated to this PR, unless I misunderstood.
"the red is also far too strong I cannot read text in it." Do you mean the Dark theme new red ?

@Ducasse
Copy link
Member

Ducasse commented Jan 31, 2020

Julien we have a conflict.

@hogoww
Copy link
Contributor Author

hogoww commented Jan 31, 2020

Should be fine now

@hogoww
Copy link
Contributor Author

hogoww commented Jan 31, 2020

Failure seems unrelated :)

@MarcusDenker MarcusDenker merged commit 73b7ab8 into pharo-project:Pharo9.0 Feb 5, 2020
@hogoww hogoww deleted the drTestColor branch October 28, 2022 12:00
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.

6 participants