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 URL Parameter to Show Block IDs in Tooltip #9803

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

thsparks
Copy link
Contributor

This adds the tooltipBlockIds URL parameter. When set to 1 in the URL (?tooltipBlockIds=1), blocks will include their block id in the tooltip when you hover over them.

I'm doubtful that an exported bool directly inside pxt.blocks is the best way to store this, but I figured I'd send the PR anyway and get feedback. If anyone has thoughts on a better approach, please let me know!

…lock ids to the tooltip text for each block.

I'm doubtful that an exported bool directly inside pxt.blocks is the best way to store this. Will need to run that by the team to get feedback.
@thsparks thsparks requested a review from a team December 20, 2023 19:28
@@ -716,7 +716,8 @@ namespace pxt.blocks {
block.setPreviousStatement(!(hasHandlers && !fn.attributes.handlerStatement) && fn.retType == "void");
block.setNextStatement(!(hasHandlers && !fn.attributes.handlerStatement) && fn.retType == "void");

block.setTooltip(/^__/.test(fn.namespace) ? "" : fn.attributes.jsDoc);
block.setTooltip((/^__/.test(fn.namespace) ? "" : fn.attributes.jsDoc) + (pxt.blocks.showBlockIdInTooltip ? " (id: '" + fn.attributes.blockId + "')" : ""));
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a question about validation since I know that this tooltip and the flag are largely for help with investigation/debugging there. For our blocks exist validation currently, are we only looking at block id? Just wanted to make sure that there weren't other identifiers that we were using to look for blocks because I remember in some docs work that I did early on, that qName was used, but I might be misremembering it's significance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe we check against the id and nothing else, though we could potentially change that in the future if needed.

Copy link
Contributor

@srietkerk srietkerk left a comment

Choose a reason for hiding this comment

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

LGTM

@thsparks thsparks merged commit 3751b5a into master Jan 18, 2024
7 checks passed
@thsparks thsparks deleted the thsparks/block_id_on_hover branch January 18, 2024 01:24
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.

2 participants