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

Adding key action to move entry boundary to cursour position under multiple entries editing mode #45

Closed
RibosomeK opened this issue Aug 27, 2024 · 7 comments

Comments

@RibosomeK
Copy link

Why

In oto, there is default key action to set certain parameter to cursor position, beacuse of its fields are set. While in textgrid is different, with no preset fields and fields numbers, so the default key action would not work. I want to add a similar feature to move parameter line by key action.

What

In details, two key action, Move Boundary Forward To Cursor Position and Move Boundary Backward To Cursor Position would be added, and what they do, is base on current cursor position, move the boundary line after or before the cursor position (corresponding move forward and backward) to the cursor position.

I wrote a little demo to show how it may works (only move forward is implemented)
GIF
In demostration, I use a key action to move three parameter lines forward to current cursor position. If this feature is welcome, I would like to complete it and notify with a pull request.

Where

This change only work under multiple entries mode, which means single entry mode like editing oto would not be affected. Currently I only test on textgrid, but I assume labeler that edit lab file would also work.
Customize these key action and corrrespondent Simplified Chinese translation would also be added.

How

To implentment the main functionalty, I would modify getUpdatedEntriesByKeyAction in com.sdercolin.vlabeler.ui.edior.labeler.marker.MarkerState.kt, when the size of entries is more than 1 it would excute some other code, instead of early return. Also, to locate entry base on cursor position, anthor function would be added in MarkerState class, which would return the entry and its index. Beside, fix-drag would not be consider since this has little pratical meaning.

Additional context

Please let me know if there is any problem or lack of consideration, thanks.

@sdercolin
Copy link
Owner

Thank you for your idea and work!
I think your idea looks good, except for the wording:
forward and backward might not be clear enough. Personally, what you are doing in the demo looks like moving backward to me.
How about using left/right? e.g. Move the closest boundary in the left to cursor position (please ignore this if you have a better choice)

Another idea:
Is there any chance / does it make sense to just use the existing SetValueX actions instead?
Currently, they are only used in the single-edit mode, simply because it takes some extra cost to figure out which entry it should refer to in the multiple-edit mode. But since you are implementing this part, would it be possible to just support all SetValueX actions in the multiple-edit mode? The start/end boundaries should be mapped to 0 and fieldCount + 1 (ASIS).

@RibosomeK
Copy link
Author

For naming, maybe Move Left/Right Boundary To Cursor Position could be consise version of your suggestion that shows in key action setting dialog.

And I would say it kindda make sense to correspond SetValue1 with Move Left , and SetValue2 with Move Right if consider start and end boundaries without potential field in that entry. I'm not pretty sure wheather there is this kind of situation that entries can have fields with shortcuts in multiple-edit mode. And fixed-drag might also need be considered under the situation. I would like to do some experiment first to see if I can handle it. I will let you know the result ASAP.

If you have any other advice or suggestion, please let me know.

@RibosomeK
Copy link
Author

After some attempts, and try editing with this new key action, I found out that maybe seperate SetValueX and Move Left/Right Boundary To Cursor Position is semanticly better. When excuting SetValueX, you can move the parameter line from both direction relative to cursor position (basicly anywhere you want), while latter can only move from one direciton to the cursor. So maybe adding new key action would be more appropriate? I guess?

And SetValueX could also be implemented in multiple-edit mode if I can get the index of current entry, but it seems like this value cannot be access in MakerState and it is stored in EditorState.project. Is there anyway to accessing this value? Or is there a better way to do SetValueX?

@sdercolin
Copy link
Owner

I think it's good to have both

  • new Move Left/Right actions
  • support SetValueX in multi-edit mode

About how to access the current entry:
In multi-edit mode, the MarkerState.entries will be assigned with a list of all entries that are currently being edited.
The extra info we need here, is "which entry is the current cursor on".
The project.currentModule.currentIndex has less meaning here, because user can edit any entries without selecting them. Instead, we should focus on the cursor position.
Currently, we have a position saved in MarkerState.cursorState (type MarkerCursorState), but it's only maintained when the Cursor tool is being used. So we may need to modify the update logic to make it always updated. For example, we can move the position outside MarkerCursorState as a direct property of MarkerState (here, note that it needs to be a MutableState<Float>).
Once we have position updated all the time, we can use its value to find which entry in entries has currently covered the cursor position during the execution of the actions.
I hope this answers your question.

@RibosomeK
Copy link
Author

Sorry for the late reply due to some personal issue and I am so so sorry. I hope it would not cause any trouble to you.
Back in this feature, I would follow your advice to have both:

  • support SetValue1-2 in multi-edit mode corresponding to left and right parameter
  • new Move actions for currentEntry

While trying to get currentIndex I add currentIndex as a direct property of MarkerState:

fun rememberMarkerState(...) {
    ...
    val currentIndex = mutableStateOf(project.currentModule.currentIndex)
    ...
}

But it would not get updated by jumping to other entry, both click on entry list or scrolling, but it could be updated after entries updated, like through SetValueX of dragging. How could I keep it update all the time?

And beside, I am also working on two other feature: click on canvas to jump to entry and border highlight, which might look like this:
Border highlight
border-highlight
Click to jump
click-to-jump

Should I open new issue for further disscusion or it is fine to disscuss here?

@colin-weng-s
Copy link
Contributor

But it would not get updated by jumping to other entry, both click on entry list or scrolling, but it could be updated after entries updated, like through SetValueX of dragging. How could I keep it update all the time?

Sorry I am not sure what you are asking. Maybe you can open a PR if you already have most of the code, and we can discuss on the PR. It will be more easy for me to help testing.

For other two features, samely, if you already have code, PR is better, if not, you can open new issues for each 😄

@sdercolin
Copy link
Owner

Being handled in PRs:
#48
#49
#50

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

No branches or pull requests

3 participants