Graduated classification w. fixed interval: fix or mitigate UX issue

[elpaso]

Fix #51687 by using a timer to detect end of typing in the interval spinbox.

[nyalldawson]

@elpaso I've proposed an alternative fix here which avoids the timer logic (and also avoids the overhead of the unwanted changed/widget update cycle) here #51720

Keen on your thoughts as to which is preferable

[elpaso]

@nyalldawson I tested your PR and it doesn't seem to fix the issue. I still get continuous updates while typing.

[nicogodet]

I downloaded Nyall's mingw build and give it a try on Monday

[nyalldawson]

@elpaso

What am I missing? here's what I see with my branch:

Peek.2023-02-06.10-56.mp4

[nicogodet]

Yeah I confirm that Nyall's PR fixes the issue

The handle of . , for the displayed value in spinbox is delayed which solves the described issue while we still get continuous update of graduated classes

[elpaso]

I tested your branch with the world layer and a costly expression which involves $area, this takes a decent amount of time to calculate when the fixed interval is small.

In that case, when you type the first digit it starts calculating and freezes the GUI even if you type fast.
I cannot see another solution than a timer to guess the "end of typing" event.

Also, the original bug of misinterpretation of decimals was due to non-english locale with comma separator for decimals.

But if you feel ok with this solution I have no objections.

[nicogodet]

I agree that the freezing UI is a bit annoying but personally, I prefer this rather than misinterpretation of decimals.
Maybe a mix of your PR ?

[elpaso]

I agree that the freezing UI is a bit annoying but personally, I prefer this rather than misinterpretation of decimals. Maybe a mix of your PR ?

Wasn't the misinterpretation of decimals fixed (or mitigated) by my PR too?

But yes, I also though of merging the two approaches.

[nicogodet]

Wasn't the misinterpretation of decimals fixed (or mitigated) by my PR too?

Not fully fixed if you stop typing and timer triggers the update

[nyalldawson]

I also think we'll need both for the best experience 👍

[elpaso]

@nyalldawson I'll cherry-pick your commit and re-test the merge if that's ok with you.

[github-actions]

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

• that all unit tests are passing

• that all comments by reviewers have been addressed

• that there is enough information for reviewers, in particular

  • link to any issues which this pull request fixes

  • add a description of workflows which this pull request fixes

  • add screenshots if applicable

• that you have written unit tests where possible
  In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
  If there is no further activity on this pull request, it will be closed in a week.

[elpaso]

@nyalldawson what do we do with this (and yours #51720 PR) ?

Summarizing:

• I say this PR mitigates the issue
• @nyalldawson and @nicogodet say Avoid updating classification widgets which the user is editing #51720 fixes the issue
• I say Avoid updating classification widgets which the user is editing #51720 is a no-op (gdb never stopped in the branch)

I honestly don't care who is right but this seems a big waste of time, should we merge both?

[nyalldawson]

I definitely think both should be merged. I'd like to see us take more caution in future of unnecessary immediate responses to keystrokes, just like you've done here, so this PR is very welcome. But the other one is also desirable because it will fix the case when a user "pauses" mid-way through entering a new value.

So let's merge this, then I'll get mine ready to merge too.