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

feat: add added the ability to customize the color of for each blocks #7435

Closed

Conversation

Grougalorasalar
Copy link

@Grougalorasalar Grougalorasalar commented Aug 26, 2023

The basics

The details

Resolves

Added the ability to customize the color of field_labels for each block.

Proposed Changes

Override applyColour for field_label and add colourFieldLabel in BlockStyle interface.

Reason for Changes

This provides a wider range of customization options, using colors such as white or yellow with field_labels written in black, for example. If the block is disabled, the text color is white.

Test Coverage

Change tests in theme_test.js file for include colourFieldLabel in expectedOutput.

Documentation

Update BlockStyle (add explanation of colourFieldLabel) and FieldLabe (add explanation of applyColour ) .

Additional Information

@google-cla
Copy link

google-cla bot commented Aug 26, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome! It looks like this is your first pull request in Blockly, so here are a couple of tips:

  • You can find tips about contributing to Blockly and how to
    validate your changes on our
    developer site.

  • All contributors must sign the Google Contributor License
    Agreement (CLA). If the google-cla bot leaves a comment on this
    PR, make sure you follow the instructions.

  • We use conventional commits
    to make versioning the package easier. Make sure your commit
    message is in the proper format
    or learn how to fix it.

  • If any of the other checks on this PR fail, you can click on
    them to learn why. It might be that your change caused a test
    failure, or that you need to double-check the
    style guide.

Thank you for opening this PR! A member of the Blockly team will review it soon.

@github-actions github-actions bot added PR: feature Adds a feature and removed PR: feature Adds a feature labels Aug 26, 2023
@BeksOmega BeksOmega requested review from BeksOmega and removed request for rachel-fenichel August 29, 2023 22:43
@BeksOmega
Copy link
Collaborator

Hello! I think you can do this with our current css selectors:

/* replace geras-renderer.classic-theme with your theme and renderer */
.geras-renderer.classic-theme .blocklyDisabled .blocklyText {
  fill: #000;
}

Does that solve your use-case?

@Grougalorasalar
Copy link
Author

Yes, but if I would a black block and white block I need white text for the first block and black text for the second block. The goal of the PR is to have a large color palette for blocks, making them more legible by changing the text color. However, we could used this : https://www.w3.org/WAI/WCAG21/Techniques/general/G18 for automatic adaption text color but we have less possibilities color for the texts blocks.

@BeksOmega
Copy link
Collaborator

Yes, but if I would a black block and white block I need white text for the first block and black text for the second block. The goal of the PR is to have a large color palette for blocks, making them more legible by changing the text color. However, we could used this : https://www.w3.org/WAI/WCAG21/Techniques/general/G18 for automatic adaption text color but we have less possibilities color for the texts blocks.

That makes sense! But I think in general as a library we want to move away from doing styling changes in JavaScript and instead have people use CSS. This allows the browser to better optimize repaints, and makes our API more maintainable.

If we applied a css class to the block that was the block's block-style name, do you think you could achieve what you want with CSS?

@BeksOmega
Copy link
Collaborator

Yes, but if I would a black block and white block I need white text for the first block and black text for the second block. The goal of the PR is to have a large color palette for blocks, making them more legible by changing the text color. However, we could used this : https://www.w3.org/WAI/WCAG21/Techniques/general/G18 for automatic adaption text color but we have less possibilities color for the texts blocks.

That makes sense! But I think in general as a library we want to move away from doing styling changes in JavaScript and instead have people use CSS. This allows the browser to better optimize repaints, and makes our API more maintainable.

If we applied a css class to the block that was the block's block-style name, do you think you could achieve what you want with CSS?

Heya @Grougalorasalar Still definitely interested in your thoughts on this!

@BeksOmega
Copy link
Collaborator

Hello @Grougalorasalar I'm going to go ahead and close this as stale, but if you still have thoughts feel free to reopen this or file an issue!

@BeksOmega BeksOmega closed this Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: feature Adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants