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

Feature/fill circle #51

Closed
wants to merge 3 commits into from
Closed

Conversation

abhaysp95
Copy link
Contributor

Feature request done for issue 48

Changelog:

  • Ability to set general.accessibility in config.toml file for viddy
  • Change to fill circle upon setting on the option of Suspend, Diff, Bell when accessibility is set to true
  • And change the foreground color to black and background color to white (for better contrast)
  • If accessibility is set to false, viddy behaves as it did before

@sachaos
Copy link
Owner

sachaos commented Jun 25, 2022

@coolabhays Thank you. Accessibility mode is great.
I consider it enough to start with that it will be a filled circle.
I would like to avoid changing the foreground color to black and the background color to white because it conflicts with the theme setting.

@sachaos
Copy link
Owner

sachaos commented Jun 28, 2022

@coolabhays I want to merge this.
If there is no comment here in 3 days, I would like to fix and merge this PR

@abhaysp95
Copy link
Contributor Author

@sachaos Yes, changing foreground and background color to black and white does stands out from the theme, but since this was for accessibility setting, it was not so bad payoff. Also, since there are different types of colour-blindness, white-black combination seemed a pretty valid choice.
Also, mention the usage (maybe in wiki for viddy) all the different color value settings like you used for viper configuration under [color], as going through code I wasn't sure, like what's secondary_text, tertiary_text, graphics etc., as it would help in using them in development.
The color combination of for diff which is I think [white:green]' by default, is also not a good combination for colour-blindness.

What I think is that accessibility should be a toggle switch, which should set necessary settings on the little payoff for the theme (which will not be of much usage for them anyway). Do tell what do you think ?

@abhaysp95 abhaysp95 closed this Jun 30, 2022
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.

2 participants