Skip to content

fix: remove code of removed colour toolbox #2308

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.

Sign up for GitHub

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

Merged
maribethb merged 2 commits into from
Apr 4, 2024
Merged

fix: remove code of removed colour toolbox #2308

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fa76ee6
fix: devsite init script
mobyw Apr 4, 2024
cfe5603
fix: devsite style
mobyw Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions examples/devsite-demo/script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,9 @@ function init() {
toolboxJson['contents'][2].name = getMsg('Math');
toolboxJson['contents'][3].name = getMsg('Text');
toolboxJson['contents'][4].name = getMsg('Lists');
toolboxJson['contents'][5].name = getMsg('Colour');
// Separator.
toolboxJson['contents'][7].name = getMsg('Variables');
Copy link
Contributor

@BeksOmega BeksOmega Apr 4, 2024

Choose a reason for hiding this comment

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

@maribethb do you know why we're doing this instead of using Blockly's normal localization?

Copy link
Contributor

@maribethb maribethb Apr 4, 2024

Choose a reason for hiding this comment

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

Toolboxes don't really have support for dynamic messages afaik. I could see a couple different ways of solving this but I'm not sure which method you're referring to as "normal."

If you want history though, you might have to check with Neil. This is the old demo that's been on devsite for forever and I'm not sure where it came from before we moved it to samples.

Copy link
Contributor

@BeksOmega BeksOmega Apr 4, 2024

Choose a reason for hiding this comment

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

They support replacing message references in category names. Although I guess we don't document this anywhere?

@NeilFraser do you have context for this?

Copy link
Contributor

@maribethb maribethb Apr 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

TIL, I thought you had to do this manually. I thought one of the old demos was doing something similar but I can't find it.

toolboxJson['contents'][8].name = getMsg('Procedures');
toolboxJson['contents'][6].name = getMsg('Variables');
toolboxJson['contents'][7].name = getMsg('Procedures');

// Inject default variable name.
// https://github.com/google/blockly/issues/5238
Expand Down
5 changes: 2 additions & 3 deletions examples/devsite-landing-demo/script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,9 @@ function init() {
toolboxJson['contents'][2].name = getMsg('Math');
toolboxJson['contents'][3].name = getMsg('Text');
toolboxJson['contents'][4].name = getMsg('Lists');
toolboxJson['contents'][5].name = getMsg('Colour');
// Separator.
toolboxJson['contents'][7].name = getMsg('Variables');
toolboxJson['contents'][8].name = getMsg('Procedures');
toolboxJson['contents'][6].name = getMsg('Variables');
toolboxJson['contents'][7].name = getMsg('Procedures');

// Inject default variable name.
// https://github.com/google/blockly/issues/5238
Expand Down
8 changes: 2 additions & 6 deletions examples/devsite-landing-demo/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,11 @@ option {
border-left: 15px solid #4db6ac !important;
}

#blockly-5 {
border-left: 15px solid #ffcdd2 !important;
}

#blockly-7 {
#blockly-6 {
border-left: 15px solid #ef9a9a !important;
}

#blockly-8 {
#blockly-7 {
border-left: 15px solid #d7ccc8 !important;
}
Copy link
Contributor

@BeksOmega BeksOmega Apr 4, 2024

Choose a reason for hiding this comment

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

I don't think these CSS IDs are technically supported. We should be using a custom toolbox, or category CSS.

Copy link
Contributor Author

@mobyw mobyw Apr 4, 2024

Choose a reason for hiding this comment

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

I tried using category css and it works.

Should I update on this PR or open another one when this is done?

Copy link
Contributor

@BeksOmega BeksOmega Apr 4, 2024

Choose a reason for hiding this comment

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

You can make updates on this PR! Thanks for testing that out :D

Copy link
Contributor

@maribethb maribethb Apr 4, 2024

Choose a reason for hiding this comment

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

Hey @mobyw since this fixes the issue with the live demo we'll actually go ahead and merge it now so we can make sure the demo is running. If you're still up for fixing the css to use the category CSS, that would be great and we'd love a follow up PR! Thanks!


Expand Down