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

[blockly] Add multi-select feature #2419

Merged
merged 4 commits into from Feb 28, 2024

Conversation

jimtng
Copy link
Contributor

@jimtng jimtng commented Feb 27, 2024

  • Shift + Click to select multiple blocks
  • Also shift + mouse drag to multi-select
  • Currently multiselect doesn't work on mobile (AFAIK) but copy/pasting even across different scripts works
  • Provides cross tab copy/paste, including copy/pasting multiple selected blocks.

The multi-select plugin conflicts with the cross-tab-copy-paste plugin and makes it redundant, so it is removed in this PR

Also in this PR, make two fingers do panning instead of zooming.

These have been split into three commits for granularity so please don't squash them.

Resolve #2417

@jimtng jimtng requested a review from a team as a code owner February 27, 2024 09:36
Copy link

relativeci bot commented Feb 27, 2024

Job #1728: Bundle Size — 11.11MiB (+0.74%).

f4bd025(current) vs 5997c28 main#1726(baseline)

Warning

Bundle contains 19 duplicate packages – View duplicate packages

Warning

Bundle introduced 2 new packages: @mit-app-inventor/blockly-plugin-workspace-multiselect, dragselect – View changed packages

Bundle metrics  Change 4 changes Regression 1 regression Improvement 1 improvement
                 Current
Job #1728
     Baseline
Job #1726
No change  Initial JS 1.85MiB 1.85MiB
No change  Initial CSS 607.92KiB 607.92KiB
Change  Cache Invalidation 18.48% 17.29%
No change  Chunks 220 220
No change  Assets 242 242
Change  Modules 3097(+0.23%) 3090
No change  Duplicate Modules 164 164
Improvement  Duplicate Code 1.75%(-1.13%) 1.77%
Regression  Packages 151(+0.67%) 150
No change  Duplicate Packages 18 18
Bundle size by type  Change 1 change Regression 1 regression
                 Current
Job #1728
     Baseline
Job #1726
Regression  JS 9.3MiB (+0.88%) 9.22MiB
Not changed  CSS 889.4KiB 889.4KiB
Not changed  Fonts 526.1KiB 526.1KiB
Not changed  Media 295.6KiB 295.6KiB
Not changed  IMG 140.74KiB 140.74KiB
Not changed  HTML 1.24KiB 1.24KiB
Not changed  Other 871B 871B

View job #1728 reportView jimtng:blockly-multi-select branch activityView project dashboard

@rkoshak
Copy link

rkoshak commented Feb 27, 2024

Thank you!

This was really annoying when I was trying to do some testing of my block library.

@stefan-hoehn
Copy link
Contributor

Though, I am a bit(!) hesitant to add it. I also saw it as wel but note that is not supported by the blockly maintainers.

image

On the other hand, app inventor has a huge amount of resources, so I think it won't be an issue.
I will give it try and test it first, though

@stefan-hoehn
Copy link
Contributor

I also need to check closely that it removes the cross-tab-plugin.
Jimmy, please keep in mind that it is not only about adding new features but they also should be consistent what the users have used in the past. And it MUST be documented as well (sometimes that takes more time than the actual implementation).
So without having looked at it in detail yet, I am kind of worried because of the cross-rule-copy/paste.

@stefan-hoehn
Copy link
Contributor

So copy and paste across rules still work. That's good.

But when you shift-click blocks and then do a copy of them and exception is thrown in the background and you can't go on except that you have the blocks clinging to your mouse:

image

See the attached

error.mov

@jimtng
Copy link
Contributor Author

jimtng commented Feb 28, 2024

@stefan-hoehn I've updated a patch although I'll open a PR on the multi-select repo to have it fixed there too. But this means we can merge this now instead of waiting.

This is fixed in v0.1.12 which I have updated in this PR. It's added as a separate commit for you to see the changes, but I can squash it into the corresponding commit before merging.

Could you please test it again?

@jimtng jimtng force-pushed the blockly-multi-select branch 2 times, most recently from d9be766 to 2bc736e Compare February 28, 2024 03:53
@florian-h05 florian-h05 added enhancement New feature or request main ui Main UI labels Feb 28, 2024
@stefan-hoehn
Copy link
Contributor

stefan-hoehn commented Feb 28, 2024

Ok, I tested it again and from what I see it looks good.

Can you please not only provide this PR but also the PR that updates the docs accordingly 🙏🏻, so the users are not confused as we had "cross rule copy" before and now the new behaviour. Also multiselect should be described.

So in particular https://next.openhab.org/docs/configuration/blockly/rules-blockly-before-using.html#cross-rule-copy-paste should be updated at best with a new animated gif that explains how to use the new functionality.

Copy link
Contributor

@florian-h05 florian-h05 left a comment

Choose a reason for hiding this comment

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

LGTM. I just had a look at it, and this is a really nice feature!

It conflicts with the multi-select plugin and the same functionality is provided by the multi-select plugin

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
florian-h05 pushed a commit to jimtng/openhab-webui that referenced this pull request Feb 28, 2024
Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
@florian-h05 florian-h05 added this to the 4.2 milestone Feb 28, 2024
@florian-h05
Copy link
Contributor

@jimtng FYI I updated the commit messages a bit so these contain a reference to this PR when I rebase.

@florian-h05 florian-h05 changed the title Blockly: Add multi-select feature [blockly] Add multi-select feature Feb 28, 2024
@florian-h05 florian-h05 merged commit 1d22ce7 into openhab:main Feb 28, 2024
6 checks passed
florian-h05 pushed a commit that referenced this pull request Feb 28, 2024
It conflicts with the multi-select plugin and the same functionality is provided by the multi-select plugin

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
florian-h05 pushed a commit that referenced this pull request Feb 28, 2024
Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
florian-h05 pushed a commit that referenced this pull request Feb 28, 2024
Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
jimtng added a commit to jimtng/openhab-docs that referenced this pull request Feb 29, 2024
jimtng added a commit to jimtng/openhab-docs that referenced this pull request Feb 29, 2024
jimtng added a commit to jimtng/openhab-docs that referenced this pull request Feb 29, 2024
jimtng added a commit to jimtng/openhab-docs that referenced this pull request Feb 29, 2024
openhab/openhab-webui#2419

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
jimtng added a commit to jimtng/openhab-docs that referenced this pull request Feb 29, 2024
openhab/openhab-webui#2419

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
@jimtng jimtng deleted the blockly-multi-select branch March 1, 2024 06:37
jimtng added a commit to jimtng/openhab-docs that referenced this pull request Mar 3, 2024
openhab/openhab-webui#2419

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
stefan-hoehn added a commit to openhab/openhab-docs that referenced this pull request Mar 4, 2024
openhab/openhab-webui#2419

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
Co-authored-by: stefan-hoehn <mail@stefanhoehn.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request main ui Main UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Blockly mouse/trackpad navigation
4 participants