Two long taps on shots in shot list doesn't select all shots in between if second long tap is above the first

[rsevero]

Describe the bug Two long taps on shots in shot list doesn't select all shots in between if second long tap is above the first

To Reproduce Steps to reproduce the behavior:

1. Go to shots list window
2. Long tap on a shot
3. Long tap on another shot a few shots above the first
4. See that only the two taped shots are selected and not also all shots in between

Expected behavior After the second long tap I expected that all shots in between the two taped shots were also selected as it happens when the second long tap is on a shot below the first.

Device (please complete the following information):

• Samsung Galaxy 23 FE
• Android 14
• TopoDroid 6.2.64

[marcocorvi]

this had already come up some time ago, and it is the expected behaviour.

it is described in man page 7

On Fri, May 3, 2024, 19:08 Rodrigo Severo @.***> wrote:

Describe the bug Two long taps on shots in shot list doesn't select all shots in between if second long tap is above the first

To Reproduce Steps to reproduce the behavior:

1. Go to shots list window
2. Long tap on a shot
3. Long tap on another shot a few shots above the first
4. See that only the two taped shots are selected and not also all shots in between

Expected behavior After the second long tap I expected that all shots in between the two taped shots were also selected as it happens when the second long tap is on a shot below the first.

Device (please complete the following information):

• Samsung Galaxy 23 FE
• Android 14
• TopoDroid 6.2.64

— Reply to this email directly, view it on GitHub https://github.com/marcocorvi/topodroid/issues/113, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACXMYCZ4OT4D4DWFWTX7KNTZAPAANAVCNFSM6AAAAABHF3GYGGVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI3TQMBZGIYDKMI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[rsevero]

this had already come up some time ago, and it is the expected behaviour. it is described in man page 7

I just re-read man page 7 and tested current behaviour a little more. I believe it not quite intuitive as the second long tap always affects the items above it.

I wrote a different implementation where the second long tap always affects the items between itself and the previous taped item, above or below itself.

It seems more intuitive to me. It's on commit 4944b749e

Can you check if it seems as a good alternative to you?

[marcocorvi]

it seems good.

the proposed solution requires to set/clear one end of the sequence and set/clear the other end with a longtap. a longtap set after a clear, or a longtap clear after a set, have the effect of a short tap.

you should experiment with long survey listings and the multiselection in different display mode of the survey data.

there is a minor problem: once one of the two lastpos is set, where are they both cleared?

On Fri, May 3, 2024, 22:01 Rodrigo Severo @.***> wrote:

this had already come up some time ago, and it is the expected behaviour. it is described in man page 7

I just re-read man page 7 and tested current behaviour a little more. I believe it not quite intuitive as the second long tap always affects the items above it.

I wrote a different implementation where the second long tap always affects the items between itself and the previous taped item, above or below itself.

It seems more intuitive to me. It's on commit 4944b74 https://github.com/marcocorvi/topodroid/commit/4944b749ef158e1fb6ac549171f3453032ecbb33

Can you check if it seems as a good alternative to you?

— Reply to this email directly, view it on GitHub https://github.com/marcocorvi/topodroid/issues/113#issuecomment-2093681515, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACXMYC73G6Y2ODLOLSUZWX3ZAPUIXAVCNFSM6AAAAABHF3GYGGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJTGY4DCNJRGU . You are receiving this because you commented.Message ID: @.***>

[rsevero]

there is a minor problem: once one of the two lastpos is set, where are they both cleared?

On the current code they are never both cleared but I don´t think that is a problem. Do you see some situation where that would be an issue?

[marcocorvi]

On Sun, May 5, 2024, 15:06 Rodrigo Severo @.***> wrote:

there is a minor problem: once one of the two lastpos is set, where are they both cleared?

On the current code they are never both cleared but I don´t think that is a problem. Do you see some situation where that would be an issue?

when you reenter multiselection

— Reply to this email directly, view it on GitHub https://github.com/marcocorvi/topodroid/issues/113#issuecomment-2094803585, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACXMYC7YPPRHXKGZVJ3WHB3ZAYVG3AVCNFSM6AAAAABHF3GYGGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJUHAYDGNJYGU . You are receiving this because you commented.Message ID: @.***>

[marcocorvi]

the proposed update has been merged in the main

[rsevero]

On Sun, May 5, 2024, 15:06 Rodrigo Severo @.***> wrote: there is a minor problem: once one of the two lastpos is set, where are they both cleared? On the current code they are never both cleared but I don´t think that is a problem. Do you see some situation where that would be an issue? when you reenter multiselection

I believe the reentering multiselection issue is solved with commit e163784cd.

[marcocorvi]

clear() removes the items from the adapter. my concern was about having state variables not reset to default values at the end. although it did not matter (as it was)

i added a visual feedback to highlight the last chosen shot.

On Mon, May 6, 2024 at 12:26 AM Rodrigo Severo @.***> wrote:

On Sun, May 5, 2024, 15:06 Rodrigo Severo @.***> wrote: there is a minor problem: once one of the two lastpos is set, where are they both cleared? On the current code they are never both cleared but I don´t think that is a problem. Do you see some situation where that would be an issue? when you reenter multiselection

I believe the reentering multiselection issue is solved with commit e163784 https://github.com/marcocorvi/topodroid/commit/e163784cd292b1a3df379c7f73594e7d7d045267 .

— Reply to this email directly, view it on GitHub https://github.com/marcocorvi/topodroid/issues/113#issuecomment-2094974136, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACXMYC3QXO3JQ5TD32DBR7LZA2WY7AVCNFSM6AAAAABHF3GYGGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJUHE3TIMJTGY . You are receiving this because you modified the open/close state.Message ID: @.***>

[rsevero]

clear() removes the items from the adapter. my concern was about having state variables not reset to default values at the end. although it did not matter (as it was) i added a visual feedback to highlight the last chosen shot.

Besides overriding clear() I had also included a reset of lastPosAdd and lastPosRemove in clearMultiSelect(). I saw you removed those too.

Shouldn't those be there?

[marcocorvi]

they are in the method that reset the multiselection state variables

On Mon, May 6, 2024, 11:19 Rodrigo Severo @.***> wrote:

clear() removes the items from the adapter. my concern was about having state variables not reset to default values at the end. although it did not matter (as it was) i added a visual feedback to highlight the last chosen shot. On Mon, May 6, 2024 at 12:26 AM Rodrigo Severo @.***> wrote:

Besides overriding clear() I had also included a reset of lastPosAdd and lastPosRemove in clearMultiSelect(). I saw you removed those too.

Shouldn't those be there?

— Reply to this email directly, view it on GitHub https://github.com/marcocorvi/topodroid/issues/113#issuecomment-2095535335, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACXMYC23T73AA4BU63BFWWLZA5DLNAVCNFSM6AAAAABHF3GYGGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJVGUZTKMZTGU . You are receiving this because you modified the open/close state.Message ID: @.***>

[rsevero]

clear() removes the items from the adapter. my concern was about having state variables not reset to default values at the end.

I overrod clear() because it was the only "cleanup" code called at the end of "station swap" multiselection action.

The other, probably simplier action, would be to explicitly call clearMultiSelect() at the end of doSwapBlocksName() but I thought that, if the items are removed from the adapter, it would not be a bad idea to reset state varaibles.

Anyway, right now we are back at the situation where there is state left after swap stations. My last code had prevented that and prevented state leak on actions I tested.

[marcocorvi]

you are right. i restored the method clear()

many actions call updateDisplay which reload the list, which is heavy.

it would be nice to have more fine-grained calls, to the necessary actions. for example swap could call a refresh only on the views of the swapped blocks

On Mon, May 6, 2024, 11:28 Rodrigo Severo @.***> wrote:

clear() removes the items from the adapter. my concern was about having state variables not reset to default values at the end.

I overrod clear() because it was the only "cleanup" code called at the end of "station swap" multiselection action.

The other, probably simplier action, would be to explicitly call clearMultiSelect() at the end of doSwapBlocksName() but I thought that, if the items are removed from the adapter, it would not be a bad idea to reset state varaibles.

Anyway, right now we are back at the situation where there is state left after swap stations. My last code had prevented that and prevented state leak on actions I tested.

— Reply to this email directly, view it on GitHub https://github.com/marcocorvi/topodroid/issues/113#issuecomment-2095551186, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACXMYC34GSBU6ZW4Z3XCVRDZA5ENRAVCNFSM6AAAAABHF3GYGGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJVGU2TCMJYGY . You are receiving this because you modified the open/close state.Message ID: @.***>