Add a feature to hide translated files on the Project tab
ClosedPublic

Authored by sdepiets on Jul 19 2018, 2:19 AM.

Details

Summary

This patch adds a feature to hide fully translated files and folders on the project tab.
Three ways to do that :

  • In the menu bar
  • In the "Go" menu
  • With shortcut Ctrl+T

FEATURE: 224179

Diff Detail

Repository
R456 Lokalize
Branch
hide_translated_files_toggle (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 1059
Build 1072: arc lint + arc unit
sdepiets requested review of this revision.Jul 19 2018, 2:19 AM
sdepiets created this revision.
mlaurent requested changes to this revision.Jul 19 2018, 5:47 AM
mlaurent added inline comments.
src/project/projectmanagerui.rc
2

increase version when you add new entry in rc file

src/project/projecttab.cpp
31

why ? you don't add new debug in this file no ?

186

it's better to use new connect api.

src/project/projectwidget.cpp
155

coding style:
::foo()
{
...
}

This revision now requires changes to proceed.Jul 19 2018, 5:47 AM
sdepiets updated this revision to Diff 38056.Jul 19 2018, 6:04 AM
sdepiets marked 4 inline comments as done.

Changes based on inline comments

aacid added inline comments.Jul 21 2018, 4:31 PM
src/project/projectmodel.cpp
719

What's the difference between this and FuzzyUntrCountRole ?

src/project/projectwidget.cpp
155

You still need to move the { down.

518

Can you try with invalidateFilter ? Reading the documentation it seems it would make more sense.

Also IMHO it makes more sense to call this inside toggletranslatedfiles than here.

sdepiets updated this revision to Diff 38191.Jul 21 2018, 11:22 PM
sdepiets marked an inline comment as done.

Style and invalidateFilter call fix

huftis requested changes to this revision.Jul 22 2018, 8:28 AM
huftis added a subscriber: huftis.

The patch adds a toolbar item, but it’s missing an icon. Could you add one? I think the one named hide_table_row will work well.

This revision now requires changes to proceed.Jul 22 2018, 8:28 AM

Also make sure to mention bug #224179 in the revision summary, so that it will automatically be closed when you commit the revision. See https://community.kde.org/Infrastructure/Phabricator#Add_special_keywords for instructions.

sdepiets edited the summary of this revision. (Show Details)Jul 23 2018, 1:09 AM
sdepiets updated this revision to Diff 38219.Jul 23 2018, 2:20 AM
sdepiets marked 2 inline comments as done.

Add an icon for the menu entry

huftis accepted this revision.Jul 23 2018, 11:24 AM

I have done some testing, and the patch seems to work fine. Thanks for adding this very useful feature!

Thanks indeed. Very useful! :)

aacid added inline comments.Jul 23 2018, 1:58 PM
src/project/projectmodel.cpp
719

You marked this as done but didn't give me an answer?

sdepiets added inline comments.Jul 23 2018, 2:00 PM
src/project/projectmodel.cpp
719

FuzzyUntrCountRole only returns non-zero values for the .po files, not the folders (it is used to move from one file to the next, ignoring the folders).

719

Answer above, it was stuck in "unsubmitted"

aacid added inline comments.Jul 23 2018, 6:28 PM
src/project/projectmanagerui.rc
17

Why is this on the "Go" menu instead of on the "Settings" menu? To me it seems it would make more sense in settings.

src/project/projecttab.cpp
183

it also hides folders, though adding it probably makes it too long (at least for the toolbar).

I think there's two ways

a)
action->setText(i18nc("@action:inmenu","Hide fully translated files and folders"));
action->setIconText(i18nc("@action:inmenu","Hide fully translated files"));

This way the toolbar doesn't have the "and folders" but the menu does, saves some space on the toolbar but not on the menu since on the menu is less needed (though it can be a bit confusing to users the fact that there's two different texts for the same action)

b)
action->setText(i18nc("@action:inmenu","Hide fully translated files"));
action->setToolTip(i18nc("@action:inmenu","Hide fully translated files and folders"));

So the tooltip on the toolbar gives some more info.

What do you think? Or am i making a big deal of nothing? πŸ˜„

sdepiets updated this revision to Diff 38280.Jul 24 2018, 1:34 AM

Add tooltip and move menu item to settings

sdepiets marked an inline comment as done.Jul 24 2018, 1:37 AM
sdepiets added inline comments.
src/project/projectmanagerui.rc
17

ok, it could be in Project as well, I don't have a strong preference on this one so I'll follow your advice.

src/project/projecttab.cpp
183

Why not both ?

I was thinking of something like "items" instead of "files and folders" but it would be more confusing, some people may think it would hide the translated strings within the files.

adrianchavesfernandez added inline comments.
src/project/projecttab.cpp
183

I donΒ΄t think it is a bad idea to use "files", "items" or "content" on the text as long as the tooltip clarifies. Also, on the menu entry I would consider even removing the "fully" part, or replace "fully translated" by something shorter, like "complete" or "done".

sdepiets updated this revision to Diff 38292.Jul 24 2018, 6:18 AM
sdepiets marked an inline comment as done.

Change of tooltip/menuitem text

sdepiets added inline comments.Jul 24 2018, 6:19 AM
src/project/projecttab.cpp
183

So "Hide completed items" for the text and "Hide fully translated files and folders" for the tooltip should be ok for everyone I hope.

Should the state of the action be remembered between runs?

Should the state of the action be remembered between runs?

I didn't plan that, the quick search project field isn't saved between runs.
Performance wise it would not change anything to launch the program with the filter on.

aacid accepted this revision.Jul 26 2018, 5:21 PM

Should the state of the action be remembered between runs?

I didn't plan that, the quick search project field isn't saved between runs.
Performance wise it would not change anything to launch the program with the filter on.

Ok, fair enough, let's see if someone complains :)

This revision was not accepted when it landed; it landed in state Needs Review.Jul 27 2018, 2:14 AM
This revision was automatically updated to reflect the committed changes.