-
Notifications
You must be signed in to change notification settings - Fork 580
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
Hide Toolbox in Minecraft Python Hour of Code #10234
Changes from all commits
c370490
1e10641
2f188c6
f29b23e
e584653
9a5a8b9
068be00
63bf49d
b2326f4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -384,7 +384,7 @@ export class MarkedContent extends data.Component<MarkedContentProps, MarkedCont | |||||
// Renders inline blocks, such as "||controller: Controller||". | ||||||
private renderInlineBlocks(content: HTMLElement) { | ||||||
const { parent } = this.props; | ||||||
const hasCategories = !parent.state.tutorialOptions?.metadata?.flyoutOnly; | ||||||
const hasCategories = !parent.state.tutorialOptions?.metadata?.flyoutOnly && !parent.state.tutorialOptions?.metadata?.hideToolbox; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for making the change, I think this might fail in the blocks scenario now, though, since you said that the
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it only has categories when both of these checks are false, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh you are totally right. Sorry about that! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, that looks good! |
||||||
const inlineBlocks = pxt.Util.toArray(content.querySelectorAll(`:not(pre) > code`)) | ||||||
.map((inlineBlock: HTMLElement) => { | ||||||
const text = inlineBlock.innerText; | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is breaking the change that I made recently for getting rid of the icons for block snippets for
flyoutOnly
tutorials, based on one of the screenshots you included. To get rid of the icon, I'm checking that the metadata hasflyoutOnly
to true..pxt/webapp/src/marked.tsx
Line 387 in 4f68e8b
Maybe add, on this line, a check for if
hideToolbox
is true as well, because that would also mean that the tutorial wouldn't have categories.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Made your suggested change and updated the screenshot.