-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
feat: Parse newlines in JSON message as row separators. #6944
Conversation
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.
Generally LGTM! Mostly comments about renaming parameters :P
Wrt documentation: yes we should definitely document this. Could you file a docs issue for that (when you have time) and add it to our Q2 Github Project?
Also @maribethb the idea was that this doesn't force v10 (even though it's technically a breaking change in that it would change block rendering for people currently using \n) correct?
Another more design-related question (possibly for @maribethb ) Have we considered adding some more generic extra options to inputs that would allow them to be rendered differently? E.g. https://github.com/google/blockly/pull/3219/files#diff-e01c94caccf98f950fdbe393516b2595473c2e1f04e9f302ec62ed66549e2886R41 I believe this would allow App Inventor to implement their indented inputs (which it seems they still want) relatively easily externally. [Edit: Discussed this with Rachel and neither of us really like the solution of "just allow people to append extra data". So we either want to have this PR go in as-is, or we want to do some more designwork to see if there's a more structured way to support alternative rendering] |
I've reimplemented this as a new subclass of Input, similar to DummyInput! |
Created an issue to add documentation: #7360 |
Added input_end_row to the block factory demo. |
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.
Generally LGTM! Just a few little things =)
demos/blockfactory/blocks.js
Outdated
"previousStatement": "Input", | ||
"nextStatement": "Input", | ||
"colour": 210, | ||
"tooltip": "For adding fields at the end of a row with no " + |
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.
Nit: Update tooltip.
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 updated the tooltips for both this and dummy inputs, please take a look.
Created google/blockly-samples#1854 to deprecate the renderer plugin that accomplished something similar. |
The basics
npm run format
andnpm run lint
The details
Resolves
Fixes #6809. Interprets newline characters in the JSON message as
dummiesa new subclass of Input called EndRowInput that force any following inputs to be rendered on a new row.Proposed Changes
Behavior Before Change
Newline characters from the message are preserved, but render as regular spaces by the browser in a label field.
Behavior After Change
Newline characters are converted to special
dummyend-row inputs that force the next input to be rendered on a new line. (Also, these dummies use the same alignment as the implicit last dummy.)The common RenderInfo recognizes the new dummies and handles them appropriately.
Reason for Changes
This allows more customization of block layout, including putting inline inputs on multiple rows.
Test Coverage
Unit tests in
block_json_test.js
andblock_test.js
test token parsing and conversion to dummies. Rendering is not tested.Documentation
Information about using newline characters in JSON messages, and the new "input_end_row" JSON input type, could be added to: https://developers.google.com/blockly/guides/create-custom-blocks/define-blocks