Skip to content

Conversation

Jongkeun
Copy link
Contributor

@Jongkeun Jongkeun commented Jun 15, 2018

#189

  • TestRow
    • multi-select by ctrl
    • multi-select by shift
    • select all short-cut
    • multi-copy
    • multi-paste
    • sort by index, not click sequence

@Jongkeun Jongkeun changed the title Multi select WIP:Multi select (#189) Jun 15, 2018
select() {
this.props.select(this.props.command);
select(e) {
if (e.ctrlKey) {
Copy link
Member

Choose a reason for hiding this comment

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

Thats not gonna work on macOS because the correct modifier key for mac is metaKey.
Look at the code in handleKeyDown and use primaryKey instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now I classify onFocus and onClick.
But still I can't find the way to work appropriately. 😨
I realized why onFocus is need.
I'll make it fixed by referenced your comment.
Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This event has not working like the handleKeyDown.
Because this is not key Event.
So I added it in a different way.

select() {
this.props.select(this.props.command);
select(event) {
if (event.ctrlKey || event.metaKey ) {
Copy link
Member

Choose a reason for hiding this comment

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

This is still flaky, modifier-keys is a library I wrote myself, so we can most certainly change it to fit the case.

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 tried to use modifier-keys, but although it works fine on a keyboard event, not working on the mouse event.

Copy link
Member

Choose a reason for hiding this comment

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

Then we should change modifier-keys to work for mouse events as well.

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 couldn’t find the repository.
Is your private repository?

Copy link
Member

Choose a reason for hiding this comment

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

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 tried to fix the project.
There is no part of a shift key and that library was not much different from this code.
Even you added the validation check for MouseEvent.
If I modify the repository, I think it has a long time.
Could you fix the library, please?

this.clipboard = item;
this.clipboard.clear();
for(let i=0; i<this.selectedCommands.length; i++){
this.clipboard.push(this.selectedCommands[i]);
Copy link
Member

Choose a reason for hiding this comment

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

Why not

this.clipboard.replace(this.selectedCommands)

const newCommand = this.clipboard.clone();
this.selectedTest.test.insertCommandAt(newCommand, index);
if (this.clipboard.length && this.selectedTest.test) {
for(let i=0; i<this.clipboard.length; i++){
Copy link
Member

Choose a reason for hiding this comment

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

this.clipboard.forEach((command, clipboard, index) =>

@Jongkeun Jongkeun changed the title WIP:Multi select (#189) Multi select (#189) Jul 3, 2018
@Jongkeun
Copy link
Contributor Author

Jongkeun commented Jul 3, 2018

@corevo Please checking this PR when you have a time.

@Jongkeun
Copy link
Contributor Author

@corevo When will this be merged?

@corevo
Copy link
Member

corevo commented Jul 29, 2018

Part from the gif, it is easy to click around and react an unwanted behavior (wrong rows being selected).
jul-29-2018 11-23-46

Copy link
Member

@corevo corevo left a comment

Choose a reason for hiding this comment

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

See comment

@Jongkeun
Copy link
Contributor Author

Jongkeun commented Aug 6, 2018

I can't reproduce this case.
Could you show me the error log at developer console?

@corevo corevo mentioned this pull request Aug 20, 2018
@corevo
Copy link
Member

corevo commented Aug 28, 2018

There's a bug in the dnd that I fixed at a later commit, if you can just pull those changes so that I can make sure multiple commands dnd works.
Also when clicking a command, and then the one below it and clicking the arrow up key, the focus is lost, and arrow navigation is broken.

@Jongkeun Jongkeun closed this Sep 11, 2018
@saradhit
Copy link

saradhit commented Oct 1, 2018

@corevo Is Multi Select issue fixed!!! I am not able to select multiple lines at a time. I am using IDE V 3.3.1, FF V 62.0.2 (32bit) and windows10 EE. Let me know if i am missing something.

@dvd900
Copy link
Contributor

dvd900 commented Feb 28, 2019

@Jongkeun Hey!! do you mind if pull code from this branch and try again?
@corevo any feed back for someone who's going to pursue this feature?

@tourdedave
Copy link

If there is a problem, please open a new issue.

https://github.com/SeleniumHQ/selenium-ide/issues/new/choose

@Jongkeun
Copy link
Contributor Author

Jongkeun commented Mar 4, 2019

@dvd900 Sure. But it has a performance problem also.
When there are 1000 commands, there is a slight delay.

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.

5 participants