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

mainwin: Add support for shift+dblClick to insert syllable or chord #204

Merged
merged 3 commits into from
Nov 22, 2023

Conversation

matevz
Copy link
Member

@matevz matevz commented Jan 16, 2023

Part of #84:

  • shift+double click now prepends new syllable, chordname, function mark or figure bass mark in front of the element
  • also, if syllable, chordname or figured bass mark button is selected (Insert mode) the element is prepended in the same way (to edit an existing element click / double-click on it)
  • refactors repositSyllables(), repositFunctions etc. to common repositionElements which is now virutal in CAContext
  • refactors addEmptySyllable(), addEmptyFunctionMark() etc. to a common addEmptyElement which is now virtual in CAContext
  • adds clickTimerActivated() to CAScoreView so we can read whether mouse press can be part of doubleclick or tripleclick before doing any other actions in mainwin
  • does not hide the Insert toolbar anymore when in Insert mode. This may have caused that the user could not untoggle the button and exit the insert mode
  • fixes score view vertical shifting for 2 pixels on most desktops when QTextEdit is shown in the top toolbar or not

@matevz matevz force-pushed the matevz/feature/insert-syllables-chords branch from f333515 to 46322c6 Compare November 19, 2023 17:49
@matevz matevz requested a review from suamor November 19, 2023 17:55
CAFiguredBassMark *newElt = new CAFiguredBassMark(this, timeStart, 1);
int i;
for (i = 0; i < _figuredBassMarkList.size() && _figuredBassMarkList[i]->timeStart() < timeStart; i++)
;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I checked and this is old code, but was / is there a plan to add more code?
Else for loop could be removed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, the for loop here is to compute the value of i (it's declared outside of the for block), because we have no more intelligent way of computing the position than linearly scanning the list. We could do binary search in the future though.

Copy link
Collaborator

@suamor suamor left a comment

Choose a reason for hiding this comment

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

Addedd one comment about old code, that could be optimized.

@suamor
Copy link
Collaborator

suamor commented Nov 21, 2023

We should make use of Unit Test generation features from github.

@matevz matevz merged commit 4303129 into main Nov 22, 2023
2 checks passed
@matevz matevz deleted the matevz/feature/insert-syllables-chords branch November 22, 2023 09:19
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.

2 participants