DictionaryComboBox: Keep user preferred dictionaries on top
ClosedPublic

Authored by loh.tar on Nov 19 2018, 9:44 PM.

Details

Summary

...to ease the switch between dictionaries you usually need

CCBUG: 302689

Test Plan

Three shots how it looks in Kate

Normal size

Expaned to showcase the included splitter...

...and how the combobox looks

Diff Detail

Repository
R246 Sonnet
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
loh.tar created this revision.Nov 19 2018, 9:44 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptNov 19 2018, 9:44 PM
loh.tar requested review of this revision.Nov 19 2018, 9:44 PM

The added stuff at the ConfigWidget looks to me not perfect but OK. Suggestions are welcome. Some thoughts/questions

  • Most UI member are named m_foo but some without the m_ prefix. I can't recognize a rule why and chose for my new stuff a name without the prefix due to the way how they are used later, by ui-dot (ui.foo). So my offer is to rename all uniform, let me know which you prefer
  • The DictionaryComboBox and the new QListWidget (languageList) is somehow reduntant. Would be nice to include the "default language information" into the languageList. But how? The only idea I had was to add an icon to the "default language" by a double click.
  • Changes at the selected languages appear not direct in the DictionaryComboBox, only at next run. The same rules to the list itself. It would be a little unsettled to reorder the list by each change.
  • Not investigated if in Settings::setPreferredLanguages is calling d->loader->changed() is needed. Guess it is, but why not in every setFoo function?
loh.tar edited the test plan for this revision. (Show Details)Nov 26 2018, 3:36 PM
loh.tar added a reviewer: VDG.
ngraham accepted this revision.Nov 26 2018, 4:36 PM
ngraham added a subscriber: ngraham.

Makes sense to me, visually speaking!

This revision is now accepted and ready to land.Nov 26 2018, 4:36 PM

Bug302689 - Impossible to delete unwanted dictionaries
https://bugs.kde.org/show_bug.cgi?id=302689

Does not quite fit, but relieves the symptoms. Enough to close that?

loh.tar edited the summary of this revision. (Show Details)Dec 8 2018, 6:43 AM

I think this makes sense, it improves the usability of the dictionary selection.
I see no API issues, should be BC and docs are there, too.

cullmann requested changes to this revision.Dec 9 2018, 2:48 PM

Ah, one thing: Could you add some @since 5.xxx to the new function in speller.h? I assume 5.54 would be the right one.

This revision now requires changes to proceed.Dec 9 2018, 2:48 PM

Ah, one thing: Could you add some @since 5.xxx to the new function in speller.h?

Only in speller? In settings_p.h are also two new, but there was no docu, so I also wrote nothing, but could

I assume 5.54 would be the right one.

In CMake it's currently KF5_VERSION 5.53.0 so 5.54 is the right one?

Ah, one thing: Could you add some @since 5.xxx to the new function in speller.h?

Only in speller? In settings_p.h are also two new, but there was no docu, so I also wrote nothing, but could

I think only speller.h is installed as public API, yes.

I assume 5.54 would be the right one.

In CMake it's currently KF5_VERSION 5.53.0 so 5.54 is the right one?

I think so.

loh.tar updated this revision to Diff 47194.Dec 9 2018, 5:37 PM
  • Add @since hint to speller.h
  • Add some blank lines to speller.h to be looking a little bit nicer

Um, I'm bit confused when I now look at the diff 1/2 here???

cullmann accepted this revision.Dec 9 2018, 5:45 PM

The intermediate diff looks strange, but the final diff is IMHO ok. Will apply that, thanks.

This revision is now accepted and ready to land.Dec 9 2018, 5:45 PM
This revision was automatically updated to reflect the committed changes.