Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Material Design #1076

Merged
merged 75 commits into from
Aug 20, 2015
Merged

Material Design #1076

merged 75 commits into from
Aug 20, 2015

Conversation

AndyScherzinger
Copy link
Contributor

Hi everybody,

as per discussion, I created a separate branch and I also reimplemented the material changes from scratch not using the toolbar just the actionbar.

Things done:

  • material design action bar with drawer
  • blue login screen (reverted, for CI compliance)
  • preference activity via delegation
  • material icons (from the google material icon repo)
  • Custom Progress Indication for action bar has been added since AppCompat doesn't provide this anymore
  • travis build fixed with new exploded aar AppCompat version and modified setup_env files to utilize Android-22 for compilation of the AppCompat instead of Android-16
  • Fix for Style of file/image detail download needs improvement #710

Open Issue:

  • spinner coloring for the pull to refresh is light grey (okay for the moment due to new progress indicator), can imho be ignored for now du to the Custom Progress Indicator --> DONE

@davivel @tobiasKaminsky @jancborchardt @stoyicker

@AndyScherzinger
Copy link
Contributor Author

@jancborchardt see attached some screenshots for design check :)

  • I added a progress bar below the action bar to show synch in progress.
  • most icon work is done
  • drawer icon is animated now
  • opened drawer gets closed on back pressing
  • progress bars for downloads and uploads are holo/material style now Style of file/image detail download needs improvement #710
  • orientation change is working again

So for now I would consider it ready-for-testing and code review @davivel @tobiasKaminsky @jancborchardt @stoyicker

I tested it on a Nexus 5 running 5.1.1

Since I got asked this, I work on the latest version of Android Studio + Gradle

device-2015-07-26-195459
device-2015-07-26-195215
device-2015-07-26-195238
device-2015-07-26-195317
device-2015-07-26-195404

@AndyScherzinger
Copy link
Contributor Author

@ All I fixed the build for ANT/Eclipse, at least it is working now as far as I can see.

Can anyone check this? @davivel for example, since I do not use Eclipse ADT anymore or ant - it works on my machine and in travis-ci so far.

@AndyScherzinger AndyScherzinger mentioned this pull request Jul 26, 2015
@AndyScherzinger
Copy link
Contributor Author

One additional comment, the progress bar below the action bar isn't too nicely done (grey background atm) but I couldn't find a way to add it to the action bar itself. This could be done with a toolbar since that on eis a "normal" ViewGroup but didn't find a way to use it on the Activity since when navigating to a folder the toolbar would have to show a back/up navigation which I got to work but the click event didn't do anything maybe because of the active drawer element (being) already closed. Thus is might be fixable be removing the drawer from the toolbar when navigating to a folder and read it when the user is back on the root folder. This would need some experimenting since I could get this to work on the weekend and thus moved back to the actionbar which has it's downsides like the progress bar for example.

@jancborchardt
Copy link
Member

@AndyScherzinger nice :) some things, mainly details:

  • can you try #35537A as highlight color instead of the turquoise? That’s the bright end of the gradient used on the web interface log in page.
    1. screenshot (detail download): there’s some grey shadow below the header bar? Also, grey different background color on the bottom. (There’s other stuff like: we can remove the »type of file« info as it’s obvious, and the text breaks too near the right edge of the screen, and the icon is not left-aligned with the rest of the text – but I suppose this is something for another pull request. :)
    1. screenshot (list view): the loading bar below the header has rounded corners, which makes it look disconnected from the header.
    1. screenshot (sidebar): the big dark blue rectangle and the »ownCloud« text basically duplicate the header. If we keep it at this step, the account name should be shown here. (Future: Account switcher)
    1. screenshot (settings): everything fine :)
    1. screenshot (setup): reduce the space above the ownCloud logo a bit, so that more fields are shown even when the keyboard is expanded. If the view automatically scrolls lower anyway when the view is scrolled we can make the ownCloud logo bigger – currently it looks a bit imbalanced and small.

@masensio
Copy link

Reviewing the latest changes

@supportreq
Copy link

@AndyScherzinger can we change the top status bar colour when the files are uploading or downloading and in case of any error it shows a different colour.. suggest?

@davivel
Copy link
Contributor

davivel commented Aug 20, 2015

@supportreq, not in this PR. We have to put an end to it in some point.

Please, open a new issue for your suggestion.

@AndyScherzinger
Copy link
Contributor Author

I agree with @davivel let's finish up this PR since it'll be a never ending story otherwise.

As for @supportreq's idea of using the action bar for visualizing errors in up/downloading:
I wouldn't do this, since to me it feels like a misuse for the action bar, but would suggest that such a notification to the user for up/download errors to be done in a manor the user is used to, that would imho be a classic android notification :) Like @davivel said, please open up a new issue for this matter.

@supportreq
Copy link

not sure if this is a related error but got the following when returning back from a video preview:

java.lang.NullPointerException: Attempt to invoke virtual method 'boolean android.support.v4.widget.DrawerLayout.isDrawerOpen(int)' on a null object reference
at com.owncloud.android.ui.activity.FileActivity.isDrawerOpen(FileActivity.java:319)
at com.owncloud.android.ui.activity.FileActivity.onBackPressed(FileActivity.java:311)
at com.owncloud.android.ui.preview.PreviewVideoActivity.onBackPressed(PreviewVideoActivity.java:126)
at android.app.Activity.onKeyUp(Activity.java:2453)
at android.view.KeyEvent.dispatch(KeyEvent.java:2636)
at android.app.Activity.dispatchKeyEvent(Activity.java:2704)
at android.support.v7.internal.view.WindowCallbackWrapper.dispatchKeyEvent(WindowCallbackWrapper.java:49)
at android.support.v7.app.AppCompatDelegateImplBase$AppCompatWindowCallbackBase.dispatchKeyEvent(AppCompatDelegateImplBase.java:265)
at com.android.internal.policy.impl.PhoneWindow$DecorView.dispatchKeyEvent(PhoneWindow.java:2274)
at android.view.ViewRootImpl$ViewPostImeInputStage.processKeyEvent(ViewRootImpl.java:3925)
at android.view.ViewRootImpl$ViewPostImeInputStage.onProcess(ViewRootImpl.java:3887)
at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:3456)
at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:3509)
at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:3475)
at android.view.ViewRootImpl$AsyncInputStage.forward(ViewRootImpl.java:3585)
at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:3483)
at android.view.ViewRootImpl$AsyncInputStage.apply(ViewRootImpl.java:3642)
at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:3456)
at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:3509)
at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:3475)
at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:3483)
at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:3456)
at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:3509)
at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:3475)
at android.view.ViewRootImpl$AsyncInputStage.forward(ViewRootImpl.java:3618)
at android.view.ViewRootImpl$ImeInputStage.onFinishedInputEvent(ViewRootImpl.java:3779)
at android.view.inputmethod.InputMethodManager$PendingEvent.run(InputMethodManager.java:2208)
at android.view.inputmethod.InputMethodManager.invokeFinishedInputEventCallback(InputMethodManager.java:1849)
at android.view.inputmethod.InputMethodManager.finishedInputEvent(InputMethodManager.java:1840)
at android.view.inputmethod.InputMethodManager$ImeInputEventSender.onInputEventFinished(InputMethodManager.java:2185)
at android.view.InputEventSender.dispatchInputEventFinished(InputEventSender.java:141)
at android.os.MessageQueue.nativePollOnce(Native Method)
at android.os.MessageQueue.next(MessageQueue.java:143)
at android.os.Looper.loop(Looper.java:122)
at android.app.ActivityThread.main(ActivityThread.java:5254)
at java.lang.reflect.Method.invoke(Native Method)
at java.lang.reflect.Method.invoke(Method.java:372)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:898)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:693)

@AndyScherzinger
Copy link
Contributor Author

@supportreq that definitely is a bug beeing introduced with the material changes. How woul I reproduce this? (tried an mp4 file, but the app isn't able to open this format, so I am not sure what kind of video file I need, to reproduce and fix)

@AndyScherzinger
Copy link
Contributor Author

@supportreq can you update and try again? I added an untested fix which should prevent the NullPointerException.

@masensio
Copy link

@AndyScherzinger, @supportreq
I've tested this fix and it works fine :)

@supportreq
Copy link

@AndyScherzinger works! thanks for the fix!! :)

@masensio
Copy link

👍

masensio pushed a commit that referenced this pull request Aug 20, 2015
@masensio masensio merged commit 0c7a9c3 into develop Aug 20, 2015
@masensio masensio deleted the material_toolbar branch August 20, 2015 12:15
@masensio
Copy link

PR merged and branch deleted.
@AndyScherzinger your great your is in ownCloud now!! Many thanks for your contribution!

@rperezb
Copy link

rperezb commented Aug 20, 2015

👏 👏
great team work!

@AndyScherzinger
Copy link
Contributor Author

erm, @masensio shouldn't this be merged into master?! Like @davivel defined the new branch concept?

@strugee
Copy link

strugee commented Aug 20, 2015

@AndyScherzinger yeah, fantastic work! Honestly, I don't think I've ever seen a change of this size be completed so fast. Incredible. All we could do was sit back and watch :)

@masensio
Copy link

@AndyScherzinger master branch is also updated with develop branch.

@masensio masensio mentioned this pull request Aug 20, 2015
@AndyScherzinger
Copy link
Contributor Author

@strugee ...nah, there is still #1090 and #1100 to be reviewed, tested and #1100 to be finished icon-wise. 😉

@AndyScherzinger
Copy link
Contributor Author

Thanks @rperezb @masensio @purigarcia! Glad I can contribute and looking forward to the future PRs 😃

@AndyScherzinger
Copy link
Contributor Author

next PR in line is now #1090

@jancborchardt
Copy link
Member

WOOOHOO! :D Congrats & major thanks @AndyScherzinger! Now onto #1090 and #1100. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.