- Set minimum value of spin-box to 1
- Update spinbox boundaries in rare cases where the bar is visible while document is edited
- Don't force minimum width by buttons, see similar Bug 402904
- Rename gotoRange -> m_gotoRange
- Add modified_line_up/down buttons with mouse wheel support
- Change label text to be less redundant in conjunction with the "goto-button"
- Change QLabel to button, with an action to go to line number from clipboard. My first intend was to add a clear-button to the spin box (not so easy) because paste from clipboard needs an empty field. But then I had this idea which is much more handy.
Details
- Reviewers
cullmann - Group Reviewers
KTextEditor - Commits
- R39:3b806f3b40f7: Review KateGotoBar
Diff Detail
- Lint
Lint Skipped - Unit
Unit Tests Skipped
src/dialogs/katedialogs.cpp | ||
---|---|---|
1130 | https://doc.qt.io/qt-5/qwheelevent.html#angleDelta |
I was about to move StatusBarButton into kateviewhelpers, so that this button can used elsewhere too, like here. But got stuck. However, I think such button would be handy. Perhaps also a KateViewBarLayout.
Here shot regarding 'Change label text to be less redundant in conjunction with the "goto-button"' At least in German is that old text terrible.
src/dialogs/katedialogs.cpp | ||
---|---|---|
1130 |
Why? Have now read the doc, but without a new insight.
How and why? |
src/dialogs/katedialogs.cpp | ||
---|---|---|
1130 |
I don't know what is unclear, it should be 120 not any other value. if (object == m_ModifiedUp) { m_deltaUp += event->delta(); if (m_deltaUp >= 120) { m_ModifiedUp->click(); } and so on. If not do it right you can end up in partial value when it used finer-resolution wheels and mishmash. |
src/dialogs/katedialogs.cpp | ||
---|---|---|
1130 | I think 120 would be better to use, as that is the value one should start to handle given the docs. |
- Use 120 as wheel-delta threshold
- Use member instead of static local
If not do it right you can end up in partial value when it used finer-resolution wheels and mishmash.
I have problems to see that hazard, na matter.
I like the idea to go to next modified line up / down. I am undecided about the goto line in clipboard, since I have the feeling it adds more clutter than it helps by default: I think <CTRL+G> <CTRL+V> <ENTER> is quite fast. In addition, this leads to having "Gehe zu" twice in the ui, which is a bit confusing imho. Other opinions?
src/dialogs/katedialogs.cpp | ||
---|---|---|
1099 | Please always use QString() in favor of QStringLiteral("") | |
1156 | You should also call message->setView(m_view), otherwise the message appears in all views, in case you have multiple views of a document. | |
src/dialogs/katedialogs.h | ||
107 | Could you use lowercase naming here? m_modifiedUp instead of m_ModifiedUp? |
I have the feeling it adds more clutter than it helps by default:
Code clutter or what? The functionality is lovely and I can't see in which way It may someone jar
I think <CTRL+G> <CTRL+V> <ENTER> is quite fast.
For a keyboard-virtuoso for sure. I'm one of those who use often the mouse.
this leads to having "Gehe zu" twice in the ui, which is a bit confusing imho. Other opinions?
Um, I think you misread my post. That's my comment to the current situation, without this patch(?) With patch is the pic in the test plan.
Regarding code comments, will fix it when it got in general an "OK"
Indeed I misread the screenshot - sorry for that :-) In that case, I have no objections. @cullmann your take?
I think it is a nice enhancement over the current state.
I give it an accept, but wait for the "Regarding code comments, will fix it when it got in general an "OK"" before I apply it, thanks!
- Fix typo of member vars
- Add missing setView(m_view)
- Remove FIXME hint about timer, however would a comment for me nice
- Use QString()
Things which could be improved or not
- The up/down buttons are a little bit small, have tried to give them the hight (and width) of the hight of the other buttons, but without success. Ideas?
- There is no tooltip on the spinbox and Goto button
- The spinbox will not adjust its size and value boundaries when you paste text while its shown. Try:
- New file->show bar
- Paste 10+ lines
- Spinbox keep size of one digit and can't be set to an other value as 1
I can live with that ;=)
- The spinbox will not adjust its size and value boundaries when you paste text while its shown. Try:
- New file->show bar
- Paste 10+ lines
- Spinbox keep size of one digit and can't be set to an other value as 1
Perhaps one wants to connect to the textChanged signal and update the range.
I would disconnect the textChanged in ::closed() to avoid having updates there even if the bar is hidden again (and reconnect on show).
Otherwise I think this is ready to go in.
- Connect/Disconnect
- Remove m_gotoRange->setFocus
Like this? :-/
I noticed bad behavior and have remove these setFocus, test it if its OK.
On edit was always the focus set to the spinbox.