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

Add insufficient blocks case to code validation #9647

Merged
merged 2 commits into from
Aug 21, 2023
Merged

Conversation

kimprice
Copy link
Member

The insufficient blocks case is when the tutorial step requires more than one of the same type of block and the user has at least one of that type of block, but not all of the blocks. For example, in the picture below the user needs two "variables_set" blocks but only has one. Note: this example is a made-up tutorial step as I don't think we have any current tutorials that require more than one block of the same type in a single step.

Joey already added a check for insufficient blocks here: https://github.com/microsoft/pxt/blob/master/pxtblocks/code-validation/validateBlocksExist.ts#L27
This PR just uses that check to create the appropriate code validation message to the user.
image

@kimprice kimprice requested a review from a team August 21, 2023 17:53
@kimprice
Copy link
Member Author

We don't currently have a separate case for insufficient blocks that are disabled - it'll show the same message. For example:
image

Copy link
Contributor

@thsparks thsparks left a comment

Choose a reason for hiding this comment

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

One nit, otherwise LGTM!

@@ -81,6 +82,9 @@ export class BlocksExistValidator extends CodeValidatorBase {
} else if (disabledBlocks.length > 0) {
isValid = false;
errorDescription = lf("Make sure your blocks are connected to the rest of your code like this.");
} else if (insufficientBlocks.length > 0) {
isValid = false;
errorDescription = lf("Make sure you have enough blocks in your workspace. It should look like this.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Since it will send this message if the block exists but is disconnected, perhaps we could rephrase slightly: "Make sure you have enough blocks connected to your workspace. It should look like this.

@jwunderl
Copy link
Member

(leaving a note here for other reviewers that we'll be looking at adding other authoring for specifying required blocks vs just from highlighting in a separate pr)

@kimprice kimprice merged commit ca97bf3 into master Aug 21, 2023
6 checks passed
@kimprice kimprice deleted the kim-insufBlocks branch August 21, 2023 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants