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

Better readability #37

Closed
ghost opened this issue May 12, 2019 · 19 comments
Closed

Better readability #37

ghost opened this issue May 12, 2019 · 19 comments
Labels
help wanted Extra attention is needed

Comments

@ghost
Copy link

ghost commented May 12, 2019

Make the results of code, complexity, architecture and style more readable. With the dark color, the results are hard to read.

@szepeviktor
Copy link
Contributor

Could you share a screenshot?

@nunomaduro nunomaduro added the help wanted Extra attention is needed label May 12, 2019
@ghost
Copy link
Author

ghost commented May 12, 2019

Capture

@nunomaduro
Copy link
Owner

I think that is actually a problem from your theme. Can you tell me the theme of your terminal and the terminal you use?

@ghost
Copy link
Author

ghost commented May 12, 2019

I don't have a theme in Laravel Homestead.

@stephenlake
Copy link

stephenlake commented May 18, 2019

I can't replicate this on my side, I always get natural colours:

Screenshot from 2019-05-18 12-45-30

@ghost
Copy link
Author

ghost commented May 18, 2019

@stephenlake which system do you use if you can't replicate this?

@stephenlake
Copy link

A little bit of everything, except Windoze. Judging by that font style from your terminal screenshot, I'm guessing you're running homestead on Windoze?

@ghost
Copy link
Author

ghost commented May 18, 2019

yes it runs on windows 10

@preliot
Copy link

preliot commented May 22, 2019

Same here. Followed https://phpinsights.com/get-started.html#within-laravel to the letter on a Windows 10 64 bits environment for a Laravel application.

Hard to read, because of contrast in the collored boxes for code, complexity, architecture and style. Virtually not readable when in 80-100 scale.

@olivernybroe
Copy link
Collaborator

We are using the ascii colors. What terminal are you using?

A suggestion could be to make a bigger contrast on the colors, like this
image

@nunomaduro
Copy link
Owner

@caneco Maybe can help here.

@nunomaduro
Copy link
Owner

No plans to work on this at this time.

@enigmatic-user
Copy link
Contributor

enigmatic-user commented Sep 14, 2022

I have the same problem on a Windows 10 (21H2) machine using Take Command v28 as terminal (same in CMD and PowerShell); it looks like in the first screenshot from @ghost.

It seems that the fg=black definition in the Console constants QUALITY, COMPLEXITY, STRUCTURE, and STYLE doesn't work correctly on Windows (at least not on my system); when I change it to fg=white, the percentages are clearly readable.

I couldn't find out yet what the problem with a black foreground is, but maybe it could be changed to white so that all users can see the percentages without using workarounds like selecting the text with the mouse.

@JustSteveKing
Copy link
Collaborator

I have the same problem on a Windows 10 (21H2) machine using Take Command v28 as terminal (same in CMD and PowerShell); it looks like in the first screenshot from @ghost.

It seems that the fg=black definition in the Console constants QUALITY, COMPLEXITY, STRUCTURE, and STYLE doesn't work correctly on Windows (at least not on my system); when I change it to fg=white, the percentages are clearly readable.

I couldn't find out yet what the problem with a black foreground is, but maybe it could be changed to white so that all users can see the percentages without using workarounds like selecting the text with the mouse.

Yeah I think unfortunately we have a tendency to only test this on mac at times. Happy to accept a PR on this?

enigmatic-user added a commit to enigmatic-user/phpinsights that referenced this issue Sep 14, 2022
…s for Code, Complexity, Achitecture, and Style from black to white because for some reason on Windows (10) systems black as foreground color doesn't seem to work (see nunomaduro#37)

Signed-off-by: Jan-Christoph Ihrens <[email protected]>
@enigmatic-user
Copy link
Contributor

Unfortunately 14 of the tests failed on my system (Windows 10 / XAMPP 8.1.6, which features PHP 8.1.6) directly after cloning the forked repository and running composer install, before I had changed anything. Not sure if this is a Windows problem, too. The tests haven't been run on GitHub after creating the PR:

First-time contributors need a maintainer to approve running workflows.

Since it's a micro-change, I don't think I can possibly have caused any problems - the same 14 tests failed after the modification.

@JustSteveKing
Copy link
Collaborator

Unfortunately 14 of the tests failed on my system (Windows 10 / XAMPP 8.1.6, which features PHP 8.1.6) directly after cloning the forked repository and running composer install, before I had changed anything. Not sure if this is a Windows problem, too. The tests haven't been run on GitHub after creating the PR:

First-time contributors need a maintainer to approve running workflows.

Since it's a micro-change, I don't think I can possibly have caused any problems - the same 14 tests failed after the modification.

Thanks for this, just running the workflows not for due diligence. Both @cmgmyr and myself are in the process of planning the next stable version which will hopefully see some performance improvements alongside stability in the testing.

As soon as these workflows have ran I will merge, from the files I can see that it is only UI changing for the terminal so it shouldn't effect anything else

JustSteveKing pushed a commit that referenced this issue Sep 15, 2022
…s for Code, Complexity, Achitecture, and Style from black to white because for some reason on Windows (10) systems black as foreground color doesn't seem to work (see #37) (#600)

Signed-off-by: Jan-Christoph Ihrens <[email protected]>

Signed-off-by: Jan-Christoph Ihrens <[email protected]>
@JustSteveKing
Copy link
Collaborator

@enigmatic-user new release includes this fix. As this is an accessibility fix I thought it made sense to get it out as soon as possible.

Thanks for your contribution!

@enigmatic-user
Copy link
Contributor

@JustSteveKing Thank you very much for merging this so quickly - and for working on this great package!

@JustSteveKing
Copy link
Collaborator

@JustSteveKing Thank you very much for merging this so quickly - and for working on this great package!

You're most welcome!

I don't do much on the package atm though! We are planning the next major release atm though 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

7 participants