Review Request: Move suggestions menu for misspelled word on top.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2097/
-----------------------------------------------------------

Review request for kdelibs.


Summary
-------

Minor fix for the Bug 194979.

Bug-report requests that "Suggestions" in the context menu of spell-checker for a word, be moved to top instead of last.

Re-ordered the menu/action-adding code in KTextEdit to make the spelling "suggestions" appear first. No new code, or strings added.


This addresses bug 194979.
    https://bugs.kde.org/show_bug.cgi?id=194979


Diffs
-----

  /trunk/KDE/kdelibs/kdeui/widgets/ktextedit.cpp 1046203

Diff: http://reviewboard.kde.org/r/2097/diff


Testing
-------

Recompiled kdelibs and the superficial change appears fine - only order has changed, nothing else.


Screenshots
-----------

Moved to top - suggestions
  http://reviewboard.kde.org/r/2097/s/256/


Thanks,

Harsh

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2097/
-----------------------------------------------------------

(Updated 2009-11-10 09:04:30.008269)


Review request for kdelibs.


Changes
-------

Added a top-level change. Not sure if putting them out of a pop-up is a good UI idea, cause it might lengthen it a lot for certain words.

Tested again, diff 2 too works fine for both count==0 and otherwise.


Summary (updated)
-------

Minor fix for the Bug 194979.

Bug-report requests that "Suggestions" in the context menu of spell-checker for a word, be moved to top instead of last.

Re-ordered the menu/action-adding code in KTextEdit to make the spelling "suggestions" appear first.

(Diff 1 keeps them in a popup, diff 2 moves them to top level - as in screenshots)


This addresses bug 194979.
    https://bugs.kde.org/show_bug.cgi?id=194979


Diffs (updated)
-----

  /trunk/KDE/kdelibs/kdeui/widgets/ktextedit.cpp 1046992

Diff: http://reviewboard.kde.org/r/2097/diff


Testing (updated)
-------

Recompiled kdelibs and the superficial change appears fine.


Screenshots
-----------

Moved to top entry - suggestions
  http://reviewboard.kde.org/r/2097/s/256/
Moved to top level - suggestions
  http://reviewboard.kde.org/r/2097/s/257/


Thanks,

Harsh

[Aaron J. Seigo]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2097/#review3003
-----------------------------------------------------------


this is indeed my biggest UI gripe with the current system as well and would like to see this action, which is almost certainly the most common reason for right clicking it, should be on top. i do have one comment below, however, about how to deal with the "too long a list" issue. once that's resolved, i think this should go in.


/trunk/KDE/kdelibs/kdeui/widgets/ktextedit.cpp
<http://reviewboard.kde.org/r/2097/#comment2465>

    to prevent it from becoming TOO large, perhaps a threshold value could be used such that the first N (10?) entries are shown in the top level of the menu and any more than that are shown in a "More Sugestions >" sub menu?
   
    this assumes that the suggestions arrive in a best-suggestion sort order.


- Aaron


On 2009-11-10 09:04:30, Harsh J wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2097/
> -----------------------------------------------------------
>
> (Updated 2009-11-10 09:04:30)
>
>
> Review request for kdelibs.
>
>
> Summary
> -------
>
> Minor fix for the Bug 194979.
>
> Bug-report requests that "Suggestions" in the context menu of spell-checker for a word, be moved to top instead of last.
>
> Re-ordered the menu/action-adding code in KTextEdit to make the spelling "suggestions" appear first.
>
> (Diff 1 keeps them in a popup, diff 2 moves them to top level - as in screenshots)
>
>
> This addresses bug 194979.
>     https://bugs.kde.org/show_bug.cgi?id=194979
>
>
> Diffs
> -----
>
>   /trunk/KDE/kdelibs/kdeui/widgets/ktextedit.cpp 1046992
>
> Diff: http://reviewboard.kde.org/r/2097/diff
>
>
> Testing
> -------
>
> Recompiled kdelibs and the superficial change appears fine.
>
>
> Screenshots
> -----------
>
> Moved to top entry - suggestions
>   http://reviewboard.kde.org/r/2097/s/256/
> Moved to top level - suggestions
>   http://reviewboard.kde.org/r/2097/s/257/
>
>
> Thanks,
>
> Harsh
>
>

> On 2009-11-10 21:12:54, Aaron Seigo wrote:
> > /trunk/KDE/kdelibs/kdeui/widgets/ktextedit.cpp, lines 632-634
> > <http://reviewboard.kde.org/r/2097/diff/2/?file=14277#file14277line632>
> >
> >     to prevent it from becoming TOO large, perhaps a threshold value could be used such that the first N (10?) entries are shown in the top level of the menu and any more than that are shown in a "More Sugestions >" sub menu?
> >    
> >     this assumes that the suggestions arrive in a best-suggestion sort order.

Sonnet's default max is 10, so there should be no need for creating a submenu, yes?

The prototype for the call "highlighter.suggestionsForWord(selectedWord)" is:

    QStringList Sonnet::Highlighter::suggestionsForWord( const QString& word, int max = 10 )


- Harsh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2097/#review3003
-----------------------------------------------------------


On 2009-11-10 09:04:30, Harsh J wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2097/
> -----------------------------------------------------------
>
> (Updated 2009-11-10 09:04:30)
>
>
> Review request for kdelibs.
>
>
> Summary
> -------
>
> Minor fix for the Bug 194979.
>
> Bug-report requests that "Suggestions" in the context menu of spell-checker for a word, be moved to top instead of last.
>
> Re-ordered the menu/action-adding code in KTextEdit to make the spelling "suggestions" appear first.
>
> (Diff 1 keeps them in a popup, diff 2 moves them to top level - as in screenshots)
>
>
> This addresses bug 194979.
>     https://bugs.kde.org/show_bug.cgi?id=194979
>
>
> Diffs
> -----
>
>   /trunk/KDE/kdelibs/kdeui/widgets/ktextedit.cpp 1046992
>
> Diff: http://reviewboard.kde.org/r/2097/diff
>
>
> Testing
> -------
>
> Recompiled kdelibs and the superficial change appears fine.
>
>
> Screenshots
> -----------
>
> Moved to top entry - suggestions
>   http://reviewboard.kde.org/r/2097/s/256/
> Moved to top level - suggestions
>   http://reviewboard.kde.org/r/2097/s/257/
>
>
> Thanks,
>
> Harsh
>
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2097/#review3028
-----------------------------------------------------------

Ship it!


Looks good to me, please commit.

- Thomas


On 2009-11-10 09:04:30, Harsh J wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2097/
> -----------------------------------------------------------
>
> (Updated 2009-11-10 09:04:30)
>
>
> Review request for kdelibs.
>
>
> Summary
> -------
>
> Minor fix for the Bug 194979.
>
> Bug-report requests that "Suggestions" in the context menu of spell-checker for a word, be moved to top instead of last.
>
> Re-ordered the menu/action-adding code in KTextEdit to make the spelling "suggestions" appear first.
>
> (Diff 1 keeps them in a popup, diff 2 moves them to top level - as in screenshots)
>
>
> This addresses bug 194979.
>     https://bugs.kde.org/show_bug.cgi?id=194979
>
>
> Diffs
> -----
>
>   /trunk/KDE/kdelibs/kdeui/widgets/ktextedit.cpp 1046992
>
> Diff: http://reviewboard.kde.org/r/2097/diff
>
>
> Testing
> -------
>
> Recompiled kdelibs and the superficial change appears fine.
>
>
> Screenshots
> -----------
>
> Moved to top entry - suggestions
>   http://reviewboard.kde.org/r/2097/s/256/
> Moved to top level - suggestions
>   http://reviewboard.kde.org/r/2097/s/257/
>
>
> Thanks,
>
> Harsh
>
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2097/#review3029
-----------------------------------------------------------

Ship it!


Looks good to me, please commit.

- Thomas


On 2009-11-10 09:04:30, Harsh J wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2097/
> -----------------------------------------------------------
>
> (Updated 2009-11-10 09:04:30)
>
>
> Review request for kdelibs.
>
>
> Summary
> -------
>
> Minor fix for the Bug 194979.
>
> Bug-report requests that "Suggestions" in the context menu of spell-checker for a word, be moved to top instead of last.
>
> Re-ordered the menu/action-adding code in KTextEdit to make the spelling "suggestions" appear first.
>
> (Diff 1 keeps them in a popup, diff 2 moves them to top level - as in screenshots)
>
>
> This addresses bug 194979.
>     https://bugs.kde.org/show_bug.cgi?id=194979
>
>
> Diffs
> -----
>
>   /trunk/KDE/kdelibs/kdeui/widgets/ktextedit.cpp 1046992
>
> Diff: http://reviewboard.kde.org/r/2097/diff
>
>
> Testing
> -------
>
> Recompiled kdelibs and the superficial change appears fine.
>
>
> Screenshots
> -----------
>
> Moved to top entry - suggestions
>   http://reviewboard.kde.org/r/2097/s/256/
> Moved to top level - suggestions
>   http://reviewboard.kde.org/r/2097/s/257/
>
>
> Thanks,
>
> Harsh
>
>

> On 2009-11-11 15:07:04, Thomas McGuire wrote:
> > Looks good to me, please commit.

Committed!

But apparently I don't have permissions to mark the related bug entry as RESOLVED - as Bugzilla reports after my commit. Could someone mark it as done?

http://websvn.kde.org/?view=rev&revision=1047622


- Harsh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2097/#review3028
-----------------------------------------------------------


On 2009-11-10 09:04:30, Harsh J wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2097/
> -----------------------------------------------------------
>
> (Updated 2009-11-10 09:04:30)
>
>
> Review request for kdelibs.
>
>
> Summary
> -------
>
> Minor fix for the Bug 194979.
>
> Bug-report requests that "Suggestions" in the context menu of spell-checker for a word, be moved to top instead of last.
>
> Re-ordered the menu/action-adding code in KTextEdit to make the spelling "suggestions" appear first.
>
> (Diff 1 keeps them in a popup, diff 2 moves them to top level - as in screenshots)
>
>
> This addresses bug 194979.
>     https://bugs.kde.org/show_bug.cgi?id=194979
>
>
> Diffs
> -----
>
>   /trunk/KDE/kdelibs/kdeui/widgets/ktextedit.cpp 1046992
>
> Diff: http://reviewboard.kde.org/r/2097/diff
>
>
> Testing
> -------
>
> Recompiled kdelibs and the superficial change appears fine.
>
>
> Screenshots
> -----------
>
> Moved to top entry - suggestions
>   http://reviewboard.kde.org/r/2097/s/256/
> Moved to top level - suggestions
>   http://reviewboard.kde.org/r/2097/s/257/
>
>
> Thanks,
>
> Harsh
>
>

On Wednesday 11 November 2009 14:23:18 Harsh J wrote:
> > On 2009-11-11 15:07:04, Thomas McGuire wrote:
> > > Looks good to me, please commit.
>
> Committed!
>
> But apparently I don't have permissions to mark the related bug entry as
>  RESOLVED - as Bugzilla reports after my commit. Could someone mark it as
>  done?

Done ;)

[Albert Astals Cid-2]

A Dimecres, 11 de novembre de 2009, Harsh J va escriure:
> > On 2009-11-11 15:07:04, Thomas McGuire wrote:
> > > Looks good to me, please commit.
>
> Committed!
>
> But apparently I don't have permissions to mark the related bug entry as
>  RESOLVED - as Bugzilla reports after my commit. Could someone mark it as
>  done?

I gave you some more bugzilla powers so that doesn't happen again.

Albert