Multi select #1176

tobiasKaminsky · 2015-09-25T15:49:37Z

As discussed in #899

TASKS:

Code review @jabarros
Changes after CR @jabarros
[QA] Create 'multiSelect' branch in QA Repo @jesmrec
[QA] Merge 'multiSelect' branch in QA Repo @jesmrec
[QA] Create test plan @jesmrec
[QA] Validate test plan @jesmrec @mcastroSG

QA:

Check updated icons #1415 - semi transparent checkmark in checked checkbox image #1424

Open bugs/improvements:

tobiasKaminsky · 2015-09-30T16:44:28Z

Ready to a very first test!
Everything should work, but the "waiting dialog" is disabled for move and delete.
I do not know if this is really necessary any longer? As a user I would assume that every option is working or if not I want to have a notification and a refreshed file list...

jancborchardt · 2015-10-11T10:02:42Z

You’re awesome @tobiasKaminsky @AndyScherzinger! :)

Please test this @stoyicker @Kernald @davivel @masensio! :)

cornzy · 2015-12-29T14:13:05Z

Just tested the new Beta version:

First time I tried multiselect (long tab) I got a NullPointerException. Unfortunately I did not save it because I thought I will get it every time again (could I read the logs anywhere?).

Now I get no exception but I have following comments:

Checkboxes on the right side are very uncommon and look ugly in combination with share-icon. Nearly every other app displays the checkbox on the left. Futhermore in the normal list view there is no need of checkboxes at all when you mark the whole item in an other color - have a look at the new google fotos app. There is also a very nice solution for multi-select and it includes the new way of multi-select by dragging first to last item.

Next: when I select a folder I searched for the "download" action. I know it is called synchronize but the synchronize icon looks much like a refresh. I always thought "why should I refresh, i want to download".

AndyScherzinger · 2016-01-07T14:06:53Z

BTW the material buttons branch has been merged into master and thus when this branch is updated to the latest master it will also be able make use of the new checkbox icons :) - I didn't pull due to not being able to resolve the conflicts.

tobiasKaminsky · 2016-01-07T19:02:10Z

I will do this once it is ready for QA

masensio · 2016-02-04T11:59:12Z

Hi @tobiasKaminsky!

Thanks for this contribution. This pull request is near of the top of priorities list, merging time is near. 😄

I'm reviewing the code, but the code hasn't the lastest changes of master branch.
Could you rebase on master for updating the code?

A first Note about the code:
At a quick view I saw that several strings.xml files have been modified, I guess that it is caused by a refactor action to change a name. As you know, when you want to add/change a string in the project we only create/change this value in values/strings.xml and the rest of languages are updated using Transifex.

Thanks in advance.

masensio · 2016-02-04T12:02:49Z

res/menu/multiple_file_actions_menu.xml

+<?xml version="1.0" encoding="utf-8"?><!--
+  ownCloud Android client application
+
+  Copyright (C) 2012  Bartek Przybylski


Please, remove this Copyright line.
Add your @author line if you want.

masensio · 2016-02-04T14:26:44Z

First quick code review done.
I'll review it more deeply the next week

AndyScherzinger · 2016-02-04T14:47:49Z

@masensio any comment on my comments? 😃
I committed fixed.

AndyScherzinger · 2016-02-04T15:12:23Z

@tobiasKaminsky whenever you update to latest master, can you then please pull in my PR #1424 because this one fixes the check marks for multi select of images in grid view 😃

Else it'll hurt usability....

AndyScherzinger · 2016-02-04T15:15:17Z

@masensio when this one gets merged please make sure that #1389 gets done right after that (housekeeping) to show the right checkboxes 😃

tobiasKaminsky · 2016-02-05T17:05:35Z

@masensio regarding transifex I know, but I did refactor and not just added a new string like I should have do. I hope it is ok anyway?

tobiasKaminsky · 2016-02-05T17:06:46Z

I have updated the PR and will check your comments on monday.

tobiasKaminsky · 2016-02-06T07:47:14Z

I have done all changes according to CR and merged #1424 into it.
(I guess then #1424 can now be closed?)

davivel · 2016-03-10T12:11:40Z

@malkomich , could you review this PR after @tobiasKaminsky update?

tobiasKaminsky · 2016-03-11T16:09:15Z

Update to latest master branch is done.

AndyScherzinger · 2016-07-12T12:45:34Z

res/menu/file_actions_menu.xml

-        android:id="@+id/action_remove_file"
-        android:title="@string/common_remove"
-        android:icon="@drawable/ic_action_delete_white"
+        android:id="@+id/action_rename_file"


remove gets substituted by rename? :S

I changed the order of the elements in the menu, moving first those I think could be used more often. But remove is still up there :)

AndyScherzinger · 2016-07-12T12:48:29Z

@davivel I get the point with the icons being misleading, at the time I just used the standard material one which especially for move are a bit confusing. As for the sizings, nice catch, yes they have be to large especially for hdpi. The "ifRoom" might have to be changes to "never" since more icons might pop up on a table since the action bar has more room and thus might then show more icons.

…showAsAction=never' when action is the only one in the overflow menu

davivel · 2016-07-12T14:32:43Z

Unfortunately, the "never" option is not really mandatory. I'm finding configurations where the system ignores the "never" value if the overflow menu is left with a single element in, and moves the lonely option to the app bar instead of the three dots.

I just pushed updated icons for download and set/unset available offline; not a great difference, but at least they are different, and 'set available offline' matches the shape of the overlay status icon.

One single action... seems too few, specially for tablets.

Thanks for your feedback, pals :)

davivel · 2016-07-12T14:41:19Z

I can confirm, even with only one element in the menu marked to be shown in the bar and the rest set to "never", Nexus 9 + Android 6 insists in drawing 5 icons in the bar.

davivel · 2016-07-12T14:48:39Z

Unset available offline, in the right side:

Download (left) Vs set available offline (rigth):

davivel · 2016-07-12T14:58:45Z

Three dots button is missing on details view for tablets, in case of mobile the button appears

Going for this

davivel · 2016-07-12T16:02:32Z

@mcastroSG , after some debugging: this bug #1176 (comment) is not related with the changes here. I've compared with master and 2.0.1, and the MENU button is broken for our app in this tablet.

Could you please open a separate bug?

I'd say this should have low priority; the configuration of this tablet (hardware MENU button + Android 4.4.2) breaks Google guidelines. Maybe there's something broken in Samsung's customizations.

davivel · 2016-07-12T16:57:27Z

OK, that should do all the sub-issues ready to test. Finally . Hope.

davivel · 2016-07-12T17:11:30Z

And a totally subjective matter: isn't prettier with the ownCloud-blue-accent in the app bar instead of the current grey?

What do you think, world?

AndyScherzinger · 2016-07-12T17:21:06Z

Both seem fine for me, I am not a UI guru... I went for the grey at the time of implementation b Tobias simply due to the fact that GMail uses it :)

mcastroSG · 2016-07-13T06:13:55Z

@davivel I´ve open a new issue for #1176 (comment) and about colors i choose blue , a little more happy 😛

davivel · 2016-07-13T06:35:14Z

@mcastroSG , could you link the new issue in the description here? Thx

mcastroSG · 2016-07-13T07:35:34Z

@davivel #1738 -- Unable to get three dots button on Samsung Galaxy Tab 3 and Android 4.4.2 on details view

mcastroSG · 2016-07-13T08:18:13Z

@davivel all test are passed, I´ve checked the icon issues and all remaining problems and it seems to be working fine, great job dude!! 👏

AndyScherzinger · 2016-07-13T08:44:52Z

@davivel should the blue bar coloring for multi select then be done in an another issue/PR? Or have you done it already somewhere? 😃

davivel · 2016-07-13T10:39:48Z

@AndyScherzinger , I just read you. Done now :)

And ready to go!

AndyScherzinger · 2016-07-13T12:03:32Z

🎉 AWESOME! 🎉

davivel · 2016-07-13T12:22:24Z

Thanks everybody. This was possible only thanks to you all, @tobiasKaminsky, @AndyScherzinger, @malkomich, @DavidWiesner, @jabarros, @jancborchardt, @cornzy, @masensio, @MTRichards, @cmonteroluque, @rperezb, @jesmrec and @mcastroSG.

Getting to the end some times is hard, but sure the result worth it. :)

rperezb · 2016-07-22T09:55:45Z

certainly the result is worthy @davivel !
it's amazing to realize how many people has been involved in this one!
@kawohl 👀

JKawohl · 2016-07-22T12:49:45Z

@awesome work guys!

jancborchardt · 2016-07-26T07:30:23Z

Yeah, good stuff! Now let’s work on getting in the other pull requests from 2014 and 2015 ;)

MTRichards · 2016-07-26T09:11:43Z

Woot!!

Matt Richards
VP Products & Markets
ownCloud, inc.

On Jul 22, 2016, at 11:56 AM, Raquel Pérez Bartolomé notifications@github.com wrote:

certainly the result is worthy @davivel !
it's amazing to realize how many people has been involved in this one!
@kawohl 👀

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

tobiasKaminsky mentioned this pull request Sep 25, 2015

Long-press should initiate multiselect, not show context menu #899

Closed

tobiasKaminsky added the 3 - To review label Oct 16, 2015

jancborchardt mentioned this pull request Dec 14, 2015

[Beta 20151213] + Sign interferes with select #1346

Closed

AndyScherzinger mentioned this pull request Dec 29, 2015

House keeping: add correct checkboxes after merges #1389

Closed

2 tasks

AndyScherzinger mentioned this pull request Jan 20, 2016

Multiselect header glitch #1434

Closed

masensio added the File list label Feb 1, 2016

przybylski force-pushed the multiSelect branch from 8b22a06 to 3eb0e55 Compare February 1, 2016 20:50

masensio reviewed Feb 4, 2016
View reviewed changes

AndyScherzinger mentioned this pull request Mar 2, 2016

Feature request: multiple delete #1490

Closed

rperezb mentioned this pull request Mar 11, 2016

MULTIPLE - SELECTION - As an OC user I want to be able to do the same action over more than 1 file/folder so that I don't have to repeat the same action several times owncloud/ios-legacy#602

Closed

4 tasks

AndyScherzinger reviewed Jul 12, 2016
View reviewed changes

Added set/unset available offline action icons for tablets ignoring '…

947814b

…showAsAction=never' when action is the only one in the overflow menu

davivel added 2 commits July 12, 2016 18:09

Removed unused dialog

90d1fd9

Recovered wait dialog for move and delete operations

bf72bc2

mcastroSG added the Approved by qa label Jul 13, 2016

Blue color for action mode

87ec088

davivel merged commit 74a5942 into master Jul 13, 2016

davivel deleted the multiSelect branch July 13, 2016 11:17

harshitbansal05 mentioned this pull request Feb 5, 2018

add select all and clear selection options in menu #2108

Closed

Multi select #1176

Multi select #1176

Conversation

tobiasKaminsky commented Sep 25, 2015 • edited by mcastroSG Loading

tobiasKaminsky commented Sep 30, 2015

jancborchardt commented Oct 11, 2015

cornzy commented Dec 29, 2015

AndyScherzinger commented Jan 7, 2016

tobiasKaminsky commented Jan 7, 2016

masensio commented Feb 4, 2016

masensio Feb 4, 2016

Choose a reason for hiding this comment

tobiasKaminsky Feb 6, 2016

Choose a reason for hiding this comment

masensio commented Feb 4, 2016

AndyScherzinger commented Feb 4, 2016

AndyScherzinger commented Feb 4, 2016

AndyScherzinger commented Feb 4, 2016

tobiasKaminsky commented Feb 5, 2016

tobiasKaminsky commented Feb 5, 2016

tobiasKaminsky commented Feb 6, 2016

davivel commented Mar 10, 2016

tobiasKaminsky commented Mar 11, 2016

AndyScherzinger Jul 12, 2016

Choose a reason for hiding this comment

davivel Jul 12, 2016

Choose a reason for hiding this comment

AndyScherzinger commented Jul 12, 2016

davivel commented Jul 12, 2016

davivel commented Jul 12, 2016

davivel commented Jul 12, 2016

davivel commented Jul 12, 2016

davivel commented Jul 12, 2016

davivel commented Jul 12, 2016 • edited Loading

davivel commented Jul 12, 2016

AndyScherzinger commented Jul 12, 2016 • edited Loading

mcastroSG commented Jul 13, 2016

davivel commented Jul 13, 2016

mcastroSG commented Jul 13, 2016 • edited Loading

mcastroSG commented Jul 13, 2016

AndyScherzinger commented Jul 13, 2016

davivel commented Jul 13, 2016

AndyScherzinger commented Jul 13, 2016

davivel commented Jul 13, 2016

rperezb commented Jul 22, 2016

JKawohl commented Jul 22, 2016

jancborchardt commented Jul 26, 2016

MTRichards commented Jul 26, 2016

tobiasKaminsky commented Sep 25, 2015 •

edited by mcastroSG

Loading

davivel commented Jul 12, 2016 •

edited

Loading

AndyScherzinger commented Jul 12, 2016 •

edited

Loading

mcastroSG commented Jul 13, 2016 •

edited

Loading