-
-
Notifications
You must be signed in to change notification settings - Fork 288
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
Inject scene variables in other boxes #1047
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1047 +/- ##
==========================================
+ Coverage 94.97% 94.99% +0.01%
==========================================
Files 524 524
Lines 6944 6950 +6
==========================================
+ Hits 6595 6602 +7
+ Misses 349 348 -1
Continue to review full report at Codecov.
|
Hi @cicoub13 @VonOx @atrovato, I would love a review from you on this PR! This is how it looks like: variable-injection.mp4Is the shortcut straightforward? (having to type " {{ " to open the variable list ?) Should we use another trigger ? Like "#" ? This feature will be extended to basically all input field in scenes, for exemple in the HTTP request it could be super useful to automatically inject a variable into a POST body :) |
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.
id: `${groupIndex}.${index}.${option.name}`, | ||
text: `${groupIndex + 1}. ${index + 1}. ${option.label}`, | ||
title: `${groupIndex + 1}. ${index + 1}. ${option.label}`, | ||
value: `${groupIndex}.${index}.${option.name}` |
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.
All these values can be computed once and stored in a const instead of cumputing them n times in the loop.
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.
mm I don't follow you
We have:
- M rows (before this row)
- Each rows has N boxes
- Each box has L variables
- Each rows has N boxes
We loop on each rows before this one, then on each box in each row, then on each variable in each box.
How could we do better?
front/src/routes/scene/edit-scene/actions/SendMessageParams.jsx
Outdated
Show resolved
Hide resolved
front/src/routes/scene/edit-scene/actions/SendMessageParams.jsx
Outdated
Show resolved
Hide resolved
@cicoub13 zIndex problem fixed! :) Thanks! |
* First commit * Use variable only when ready * Use tagify directly instead of the react version * Add test to message sending with variables * Fix tests * Optimize code & fix replaceAll bug * Fix tagify behavior * Add explanation text + fix UI bug * Put {{ in const * Fix zIndex problem
Pull Request check-list
To ensure your Pull Request can be accepted as fast as possible, make sure to review and check all of these items:
npm test
on both front/server)npm run eslint
on both front/server)npm run prettier
on both front/server)npm run compare-translations
on front)front/src/config/demo.json
) so that the demo website is working without a backend? (if needed) See https://demo.gladysassistant.com.NOTE: these things are not required to open a PR and can be done afterwards / while the PR is open.
Description of change
Please provide a description of the change here. It's always best with screenshots, so don't hesitate to add some!