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

Theme.name does not match registered name #6186

Closed
BeksOmega opened this issue Jun 3, 2022 · 2 comments
Closed

Theme.name does not match registered name #6186

BeksOmega opened this issue Jun 3, 2022 · 2 comments
Labels
component: themes help wanted External contributions actively solicited issue: bug Describes why the code or behaviour is wrong

Comments

@BeksOmega
Copy link
Collaborator

Describe the bug

When you create a new theme that is not entirely lowercase, the .name property of the theme does not match the name in the registry. The registry normalizes all names to lowercase, but the themes are not doing this.

To Reproduce

Steps to reproduce the behavior:

  1. Open the playground source.
  2. Edit the start function to include:
const theme = Blockly.Theme.defineTheme('TEST', {});
console.log(theme);
console.log(Blockly.registry.getAllItems(Blockly.registry.Type.THEME));
  1. Observe how the theme has the name 'TEST', but the item has the key 'test'

Expected behavior

The theme should also have the name 'test'

Additional context

This was found when Greg attempted to define a non-lowercase theme in the advanced playground.

@BeksOmega BeksOmega added issue: bug Describes why the code or behaviour is wrong issue: triage Issues awaiting triage by a Blockly team member help wanted External contributions actively solicited component: themes and removed issue: triage Issues awaiting triage by a Blockly team member labels Jun 3, 2022
@BeksOmega BeksOmega added this to the Bug Bash Backlog milestone Jun 4, 2022
@cpcallen
Copy link
Contributor

Is there some reason that the registry normalises to lowercase? To me that seems like the bug…

BeksOmega pushed a commit that referenced this issue Jun 23, 2022
* fix: Theme.name does not match registered name (#6186)

* chore(tests): add test for defineTheme normalizing to lower case
@BeksOmega
Copy link
Collaborator Author

Closed by #6226

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: bug Describes why the code or behaviour is wrong
Projects
None yet
Development

No branches or pull requests

2 participants