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

Themes: Background Colour #2733

Closed
alschmiedt opened this issue Jul 29, 2019 · 8 comments
Closed

Themes: Background Colour #2733

alschmiedt opened this issue Jul 29, 2019 · 8 comments
Assignees
Labels
component: themes help wanted External contributions actively solicited issue: feature request Describes a new feature and why it should be added

Comments

@alschmiedt
Copy link
Contributor

Is your feature request related to a problem? Please describe.
For some people the white background of the workspace can be uncomfortable to look at. (section 4.4)

Describe the solution you'd like
As a part of themes add a way for developers to change the workspace color.

Describe alternatives you've considered

Additional context
This would probably involve adding a new optional parameter to the Theme constructor. This is currently a CSS value so we would need to find a way to overwrite or change this when we change themes.

@alschmiedt alschmiedt added issue: feature request Describes a new feature and why it should be added issue: triage Issues awaiting triage by a Blockly team member component: themes help wanted External contributions actively solicited type: accessibility and removed issue: triage Issues awaiting triage by a Blockly team member labels Jul 29, 2019
@moniika moniika added this to the 2019_q3_release milestone Jul 30, 2019
@hilts-vaughan
Copy link

Sign me up, why not? :)

@hilts-vaughan
Copy link

Proposing the following:

  1. Add the parameters to the constructor, expose trivial config
  2. In the SVG renderer, do some refactoring of the class option and instead make it a "style specifier" which is a string that is either a. a color b. a class name. We will introduce a new util that will check if the string is a color and use that. This type of overloading is pretty typical, for example: https://phaser.io/docs/2.6.2/Phaser.Stage.html#backgroundColor

This should not break the API and give enough flexibility. Thoughts? And of course, tests.

Open q's:

  1. Are you guys writing tests in Mocha now?
  2. Should we specify an explicit value on the themes in source control? It would probably serve to document them better.

@alschmiedt
Copy link
Contributor Author

Hi there!
This sounds like a good plan.

  1. We still need to include the styles in the blocklySvg class. Are you proposing leaving the class and just overwriting the background-color attribute?
  2. Yes we are testing in Mocha right now!
  3. Yes we should add a value to the example themes. Themes are documented here: https://developers.google.com/blockly/guides/configure/web/themes but feel free to add more comments and documentation in the example themes if you think it would be useful : )

@hilts-vaughan
Copy link

  1. More like the fill, since it's an SVG attribute, but yes!

2, 3: Sounds good :)

@alschmiedt
Copy link
Contributor Author

I'm not sure how setting the fill will work with our current setup.
Currently we are using the background-color found here: https://github.com/google/blockly/blob/master/core/css.js#L117 to control the background color of the workspace.
I think this has to do with how we are creating the background grid.

@hilts-vaughan
Copy link

Thanks for the info. I'll do a bit more search before I start on the actual implementation. Are you open to draft PRs to get feedback on the design? I would be a first time contributor.

@alschmiedt
Copy link
Contributor Author

Yes definitely open to a draft PR! And welcome!

@samelhusseini
Copy link
Contributor

Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: themes help wanted External contributions actively solicited issue: feature request Describes a new feature and why it should be added
Projects
None yet
Development

No branches or pull requests

5 participants