Show number of items #83

pavlospt · 2017-12-11T14:37:35Z

Resolves #79

Added number of items label in directories showing the number of contained files:

No items when directory has 0 items
1 item when directory has 1 item
# items when directory has more items

These should also be translated I guess! Leaving this up to you to handle the translations @veniosg :)

veniosg

Apologies for the ton of comments, most should be easy fixes!

veniosg · 2017-12-11T14:56:35Z

app/src/androidTest/java/com/veniosg/dir/test/actor/assertion/SeesAssertions.java

@@ -39,6 +39,13 @@ public void fileInList(String filename) {
        )).check(matches(isDisplayed()));
    }

+    public void numOfItemsLabel(String numOfItemsLabel) {


Please rename this to fileInfoInList

veniosg · 2017-12-11T15:00:46Z

app/src/main/java/com/veniosg/dir/android/adapter/FileHolderListAdapter.java

@@ -105,7 +105,7 @@ public View getView(int position, View convertView, ViewGroup parent) {
 		holder.primaryInfo.setText(item.getName());
 		holder.secondaryInfo.setText(item.getFormattedModificationDate(convertView.getContext()));
 		// Hide directories' size as it's irrelevant if we can't recursively find it.
-		holder.tertiaryInfo.setText(item.getFile().isDirectory()? "" : item.getFormattedSize(
+		holder.tertiaryInfo.setText(item.getFile().isDirectory() ? item.getNumberOfItems(convertView.getContext()) : item.getFormattedSize(


This should be extracted to FileHolder#getSizeInfo(Context ctx)

veniosg · 2017-12-11T15:01:42Z

app/src/main/java/com/veniosg/dir/android/adapter/FileListViewHolder.java

@@ -41,7 +41,7 @@ void bind(String filePath, OnItemClickListener listener) {

        primaryInfo.setText(item.getName());
        secondaryInfo.setText(item.getFormattedModificationDate(context));
-        tertiaryInfo.setText(isDirectory ? "" : item.getFormattedSize(context, false));
+        tertiaryInfo.setText(isDirectory ? item.getNumberOfItems(context) : item.getFormattedSize(context, false));


Used here too. Ideally getNumberOfItems and getFormattedSize will now be private.

veniosg · 2017-12-11T15:02:32Z

app/src/main/java/com/veniosg/dir/mvvm/model/FileHolder.java

@@ -32,9 +33,12 @@
 import java.io.File;

 public class FileHolder implements Parcelable, Comparable<FileHolder> {
+


Please remove this space :D

veniosg · 2017-12-11T15:03:36Z

app/src/main/java/com/veniosg/dir/mvvm/model/FileHolder.java

+
+		int numOfDirectoryItems = mFile.listFiles().length;
+
+		if (numOfDirectoryItems == 0) {


This should all be handled by a single plural. This will also force a single point of return for the happy case.

@veniosg Plurals in English are a hard thing. Zero is not treated as a special case in English especially. Check this for example https://stackoverflow.com/questions/5651902/android-plurals-treatment-of-zero! Maybe I can check the current configuration or locale and only do it in English.

Oh of course, good catch! In that case, it's acceptable to keep 0 items in the zero case in English to let the system handle plural localisation. This would involve duplicating the value of the many_items resource, but OneSky is smart enough to handle that so don't worry.

veniosg · 2017-12-11T15:06:20Z

app/src/main/res/values/plurals.xml

+<?xml version="1.0" encoding="utf-8"?>
+<resources>
+  <plurals name="num_of_directory_items">
+    <item quantity="one">1 item</item>


Should still be %1$s item so it's properly localised if need be.

veniosg · 2017-12-11T15:08:35Z

app/src/main/res/values/plurals.xml

@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="utf-8"?>
+<resources>


Let's keep this file but point to strings in strings.xml. If it's allowed, this should be marked as translatable=false.

e.g. <item quantity="zero">@string/no_items</item>

veniosg · 2017-12-11T15:10:53Z

app/src/androidTest/java/com/veniosg/dir/test/acceptance/FileManagerActivityTest.java

+    private final File oneItemContainerDir = new File(testDirectoryWithOneItem, "oneItemContainerDir");
+    private final File multipleItemsContainerDir = new File(testDirectoryWithMultipleItems, "multipleItemsContainerDir");
+
+    private File oneFile = null;


Make final and initialize the same way as the rest.

Maybe also name these three something more consistent like countTestFile1/2/3

veniosg · 2017-12-11T15:13:11Z

app/src/androidTest/java/com/veniosg/dir/test/acceptance/FileManagerActivityTest.java

+    private final File testDirectoryWithOneItem = new File(sdCardDir, "testDirWithOneItem");
+    private final File testDirectoryWithMultipleItems = new File(sdCardDir, "testDirWithMultipleItems");
+
+    private final File zeroItemsContainerDir = new File(testDirectoryWithZeroItems, "zeroItemsContainerDir");


What's the difference with the files above?

veniosg · 2017-12-11T15:14:46Z

app/src/androidTest/java/com/veniosg/dir/test/acceptance/FileManagerActivityTest.java

@@ -38,6 +40,17 @@
    private final File testChildDirectory = new File(testDirectory, "testChildDir");
    private final File testCopyDestination = new File(testDirectory, "testCopyDestination");
    private final File testChildFile = new File(testDirectory, "testChildFile");
+    private final File testDirectoryWithZeroItems = new File(sdCardDir, "testDirWithZeroItems");


Please create these under testDirectory. That way you need nothing more in tearDown, everything should be cleaned up by cleanDirectory(testDirectory)

veniosg · 2017-12-12T08:41:17Z

app/src/main/java/com/veniosg/dir/android/dialog/DetailsDialog.java

@@ -143,7 +143,7 @@ protected void onPostExecute(String result) {

        @Override
        protected String doInBackground(Void... params) {
-            return mFileHolder.getFormattedSize(getActivity(), true);
+            return mFileHolder.getSizeInfo(getActivity(), true);


AFAIK this will break the details Dialog. We should still use getFormattedSize(ctx, true) here.

veniosg · 2017-12-12T08:42:54Z

app/src/main/java/com/veniosg/dir/mvvm/model/FileHolder.java

 		return Formatter.formatFileSize(c, getSizeInBytes(recursive));
 	}

+	private String getNumberOfItems(@NonNull Context context) {


Maybe rename to getFormattedNumberOfItems for consistency

Update strings, names, scopes Fix details dialog Fix test crash

veniosg · 2017-12-18T19:27:26Z

Any update on this? I'd love to include it in 1.5 :)

pavlospt · 2017-12-19T08:21:12Z

@veniosg Sorry is there a pending thing on this?

veniosg · 2017-12-19T08:23:57Z

AFAIK the new test still doesn't pass. That should be all though

pavlospt · 2017-12-19T08:47:06Z

@veniosg ok will check it when I got some free time!

veniosg · 2017-12-19T08:47:41Z

Thanks!

…

On Tue, 19 Dec 2017, 8:47 am Pavlos-Petros Tournaris, < ***@***.***> wrote: @veniosg <https://github.com/veniosg> ok will check it when I got some free time! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#83 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABmcZKj6ODohtRJEOJeRE9EcGevSGBNeks5tB3gKgaJpZM4Q9fNX> .

pavlospt · 2018-05-10T18:08:55Z

I guess it can be closed!

veniosg · 2018-05-12T18:11:20Z

It was very close to being completed. Feel free to reopen if you're keen to fix the test and get it merged :)
Thanks for the effort!

pavlospt · 2018-05-12T19:58:21Z

Ohh thought it was fixed... i guess i can find some time to fix it then!

pavlospt added 5 commits December 11, 2017 15:45

Add String literals for directory items

491424c

Add getNumberOfItems() on FileHolder

1e232b2

Use FileHolder.getNumberOfItems() to display # of directory items

4817b75

Fix formatting

75daff3

Add tests for # of items in a directory

f51ace2

veniosg suggested changes Dec 11, 2017

View reviewed changes

pavlospt and others added 2 commits December 11, 2017 18:33

Fixed issues from PR feedback

2a6261d

Format SeesAssertions

4642109

veniosg reviewed Dec 12, 2017

View reviewed changes

Prepare for merge

183a37b

Update strings, names, scopes Fix details dialog Fix test crash

pavlospt closed this May 10, 2018

pavlospt reopened this May 12, 2018

pavlospt closed this Dec 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show number of items #83

Show number of items #83

pavlospt commented Dec 11, 2017

veniosg left a comment

veniosg Dec 11, 2017

pavlospt Dec 11, 2017

veniosg Dec 11, 2017

pavlospt Dec 11, 2017

veniosg Dec 11, 2017

pavlospt Dec 11, 2017

veniosg Dec 11, 2017

pavlospt Dec 11, 2017

veniosg Dec 11, 2017

pavlospt Dec 11, 2017

veniosg Dec 11, 2017

veniosg Dec 11, 2017

pavlospt Dec 11, 2017

veniosg Dec 11, 2017

pavlospt Dec 11, 2017

veniosg Dec 11, 2017

veniosg Dec 11, 2017

pavlospt Dec 11, 2017

veniosg Dec 11, 2017

pavlospt Dec 11, 2017

veniosg Dec 11, 2017 •

edited

Loading

pavlospt Dec 11, 2017

veniosg Dec 12, 2017

veniosg Dec 12, 2017

veniosg commented Dec 18, 2017

pavlospt commented Dec 19, 2017

veniosg commented Dec 19, 2017

pavlospt commented Dec 19, 2017

veniosg commented Dec 19, 2017 via email

pavlospt commented May 10, 2018

veniosg commented May 12, 2018

pavlospt commented May 12, 2018

		@@ -32,9 +33,12 @@
		import java.io.File;

		public class FileHolder implements Parcelable, Comparable<FileHolder> {


		int numOfDirectoryItems = mFile.listFiles().length;

		if (numOfDirectoryItems == 0) {

		@@ -0,0 +1,7 @@
		<?xml version="1.0" encoding="utf-8"?>
		<resources>

Show number of items #83

Show number of items #83

Conversation

pavlospt commented Dec 11, 2017

veniosg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

veniosg Dec 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

veniosg commented Dec 18, 2017

pavlospt commented Dec 19, 2017

veniosg commented Dec 19, 2017

pavlospt commented Dec 19, 2017

veniosg commented Dec 19, 2017 via email

pavlospt commented May 10, 2018

veniosg commented May 12, 2018

pavlospt commented May 12, 2018

veniosg Dec 11, 2017 •

edited

Loading