Skip to content

Conversation

samllea1
Copy link

@samllea1 samllea1 commented Jun 9, 2025

No description provided.

@github-actions github-actions bot added the pr: new extension Pull requests that add a new extension label Jun 9, 2025
@CST1229
Copy link
Collaborator

CST1229 commented Jun 9, 2025

you don't have to create a new PR each time you make a change. just push your commits (as in, making them appear on github. if you commit with the web gui this is done automatically) and it will automatically be updated

@samllea1
Copy link
Author

samllea1 commented Jun 9, 2025

you don't have to create a new PR each time you make a change. just push your commits (as in, making them appear on github. if you commit with the web gui this is done automatically) and it will automatically be updated

oh, alr. Im new to this, everything seems so complex. Sorry

Copy link
Contributor

@Brackets-Coder Brackets-Coder left a comment

Choose a reason for hiding this comment

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

I only spent a few minutes and kinda skimmed it so I probably missed something but overall this looks really good! Is this your first PR for the repository or am I oblivious to what's happening around here?

Copy link
Contributor

@Brackets-Coder Brackets-Coder Jun 30, 2025

Choose a reason for hiding this comment

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

May I ask why you added this license in the root folder? It's not necessary...

@@ -0,0 +1,547 @@
// Name: Html Elements
// ID: htmlelements
Copy link
Contributor

@Brackets-Coder Brackets-Coder Jun 30, 2025

Choose a reason for hiding this comment

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

ID should contain your username

Suggested change
// ID: htmlelements
// ID: samllea1HTMLElements


getInfo() {
return {
id: "htmlElements",
Copy link
Contributor

@Brackets-Coder Brackets-Coder Jun 30, 2025

Choose a reason for hiding this comment

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

should also contain your username and match the ID above (case-sensitive)

Suggested change
id: "htmlElements",
id: "samllea1HTMLElements",

Comment on lines +22 to +25
name: Scratch.translate({
default: "HTML Elements",
id: "htmlElements.name",
}),
Copy link
Contributor

@Brackets-Coder Brackets-Coder Jun 30, 2025

Choose a reason for hiding this comment

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

I don't believe I've ever seen the name of an extension have a structure like this, generally speaking shouldn't it just be a translated string?

Suggested change
name: Scratch.translate({
default: "HTML Elements",
id: "htmlElements.name",
}),
name: Scratch.translate("HTML Elements"),

Copy link
Contributor

Choose a reason for hiding this comment

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

Generally the banner should be some sort of artistic representation of the purpose of your extension. Also, they shouldn't contain text as it can't easily be translated. Somebody will probably eventually make a "better" banner anyway so you might not have to worry about it but I would work on a placeholder banner without text.

@samllea1
Copy link
Author

sorry, been really busy. Im not really good with github ui so idk whats going on here but was my extension added?

@Brackets-Coder
Copy link
Contributor

Nope, it still says "2 approving reviews are required by reviewers with write access"

Copy link
Contributor

@Brackets-Coder Brackets-Coder left a comment

Choose a reason for hiding this comment

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

I've updated my previous comments to be more relevant and added a few extra as needed, just a few changes to make this ready for merging

It seems to work well, so the issues aren't necessarily the functionality.

{
opcode: "createInput",
blockType: Scratch.BlockType.COMMAND,
text: "create input with id [ID] at x: [X] y: [Y]",
Copy link
Contributor

Choose a reason for hiding this comment

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

block text should be translated

Suggested change
text: "create input with id [ID] at x: [X] y: [Y]",
text: Scratch.translate("create input with id [ID] at x: [X] y: [Y]"),

blockType: Scratch.BlockType.COMMAND,
text: "create input with id [ID] at x: [X] y: [Y]",
arguments: {
ID: { type: Scratch.ArgumentType.STRING, defaultValue: "input1" },
Copy link
Contributor

Choose a reason for hiding this comment

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

Some string argument defaultValues probably should be translated

Suggested change
ID: { type: Scratch.ArgumentType.STRING, defaultValue: "input1" },
ID: { type: Scratch.ArgumentType.STRING, defaultValue: Scratch.translate("input1") },

This and the comment above goes for all blocks.

Y: { type: Scratch.ArgumentType.NUMBER, defaultValue: 0 },
},
},
{ blockType: Scratch.BlockType.LABEL, text: "Element Actions" },
Copy link
Contributor

Choose a reason for hiding this comment

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

If your extension is large enough to have labels, I'd recommend using the block separators, but it's a personal preference.

Suggested change
{ blockType: Scratch.BlockType.LABEL, text: "Element Actions" },
"---",
{ blockType: Scratch.BlockType.LABEL, text: "Element Actions" },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: new extension Pull requests that add a new extension
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants