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

UI Improvements #445

Merged
merged 30 commits into from Nov 30, 2017
Merged

UI Improvements #445

merged 30 commits into from Nov 30, 2017

Conversation

connyduck
Copy link
Collaborator

In this branch I will experiment with some ui stuff. Feedback welcome!

@connyduck
Copy link
Collaborator Author

Do people like the SparkButtons? I don't. The animation always gets cut off, is not antialiased and annoying imho.

@connyduck
Copy link
Collaborator Author

Before:
screenshot_20171110-125258

Changes so far:
screenshot_20171110-125258x

imho looks much better

@charlag
Copy link
Collaborator

charlag commented Nov 10, 2017

@connyduck I kinda liked them at first but grew tired with time. It could be okay if they were faster, smaller and worked correctly (but they glich sometimes for me). I didn't experience any anti-aliasing or cutting problems for a long time, though. I am not sure how people will react but if there will be another kind of a callback it is fine.

@charlag
Copy link
Collaborator

charlag commented Nov 10, 2017

Nice job, by the way, looks tidy!

@charlag
Copy link
Collaborator

charlag commented Nov 10, 2017

I would get rid of the links underline too.

@connyduck
Copy link
Collaborator Author

Great idea!
Next step: improving notifications the same way and polishing the detail status.

@charlag
Copy link
Collaborator

charlag commented Nov 10, 2017

Awesome!

@charlag
Copy link
Collaborator

charlag commented Nov 10, 2017

If we're already here... Do you like horizontal animations for opening activities? I don't like them much. They would also look weird with shared elements transitions which I would like to add at some point.

Also, I still forget to focus on field automatically in search. What do you think about it?

@connyduck
Copy link
Collaborator Author

Well the activity animations don't really bother me personally but I heard it from other people as well that they should be removed.
And yes, the search field should focus automatically. This should be an easy fix.

@connyduck
Copy link
Collaborator Author

New detail statuses!
Before:
screenshot_20171111-133319

After:
screenshot_20171111-132333

Still missing: number of boosts/favs in the same colour as the icon, no boosts number when private.

@charlag
Copy link
Collaborator

charlag commented Nov 11, 2017

Awesome thank you!
I see a small problem on the picture - there's no bottom line on the expanded status.

@charlag
Copy link
Collaborator

charlag commented Nov 11, 2017

Or is it intended?

@connyduck
Copy link
Collaborator Author

I am not sure what you mean exactly? Everything is intended, the thread line would have crossed the text.

@charlag
Copy link
Collaborator

charlag commented Nov 11, 2017

Yes, you're right, my apologies. I checked and Twitter works pretty much the same way

@connyduck
Copy link
Collaborator Author

So now notifications are improved as well :)

Before:
screenshot_20171117-155839

After:
screenshot_20171117-155419

@charlag
Copy link
Collaborator

charlag commented Nov 18, 2017

Wow, this looks awesome! Thank you.

@connyduck
Copy link
Collaborator Author

Less views in layouts -> more performance ✨

Copy link
Collaborator

@charlag charlag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why inefficient RelativeLayout when there's more powerful and performant ConstraintLayout?
(BTW thanks a lot for these improvements 🙇

@connyduck
Copy link
Collaborator Author

So this is now mostly done. Should fix #117 #446 #192 and partly #429
The only thing missing is that MediaActivity still behaves strange, if a post has gifs&plain images attached.
Using Glide instead of Picasso is NOT a solution, since mastodon "gifs" are in fact mp4s.

I will have another look if I find a better spark button library

@connyduck
Copy link
Collaborator Author

So I decided that Tusky 1.4.0 should be released asap, main reason is that crash rate in play console is really high. Please review this pull request, it contains at least one fix for a crash (should have made it a separate commit). @charlag

@charlag
Copy link
Collaborator

charlag commented Nov 29, 2017

Will do! It's not too late, right?

@@ -318,14 +322,12 @@

<com.varunest.sparkbutton.SparkButton
android:id="@+id/status_reblog"
android:layout_width="40dp"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought you would keep button size the same and only make icon inside it smaller. It is okay for me, just worrying for people who claim themselves clumsy.

@@ -39,6 +39,7 @@ public void onCreate() {
Picasso.Builder builder = new Picasso.Builder(this);
builder.downloader(new OkHttp3Downloader(OkHttpUtils.getCompatibleClient()));
if (BuildConfig.DEBUG) {
builder.loggingEnabled(true);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://square.github.io/picasso/2.x/picasso/com/squareup/picasso/Picasso.Builder.html#loggingEnabled-boolean-

"WARNING: Enabling this will result in excessive object allocation. This should be only be used for debugging purposes. Do NOT pass BuildConfig.DEBUG."

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you already removed it, sorry

if (!shouldShowContentIfSpoiler && hasSpoiler) {
if (statusViewData.getMentions() != null &&
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, but I don't understand why you undid this. I've tried to match web version, there's even an issue somewhere asking for this behavior. Please, consider using this for the rest of the app.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, but why was it only done for notifications? I found it highly confusing that in notifications mentions are outside spoiler, but not in normal toots

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it was only done for notifications because I didn't have time to change it everywhere.

@charlag
Copy link
Collaborator

charlag commented Nov 29, 2017

I am not sure if it's a bug or intended behavior but when I click on the notification text it doesn't get me anywhere. I have to aim at the empty space.

@charlag
Copy link
Collaborator

charlag commented Nov 29, 2017

There's no more "delete" button in the timeline, only in the detailed view. When I tried to delete my status from there it didn't disappear from the timeline (it was on the top).

@charlag
Copy link
Collaborator

charlag commented Nov 29, 2017

I think that font is statuses is a little bit too bit. I would do -1 sp or something.
Also font in detailed status is smaller than in the regular one?

@connyduck
Copy link
Collaborator Author

connyduck commented Nov 29, 2017

I will fix the notification click area, thx for noticing.

Afaik there never was a delete button in the timeline (at least not since I am maintainer). This should be consistent, yes, but I will make a seperate issue for it. (EDIT: Can't reproduce it right now o_0) That the status does not disappear is a bug that we also have for a long time.

Well Material design recommended size for regular text is 16sp. I am now using the 15sp for some days and I really love it. I think we should add a option to change the font-size between 12-18.
I am not sure, I believe font size in detailed status only looks smaller, but is actually the same size, but will check again.

@charlag
Copy link
Collaborator

charlag commented Nov 30, 2017

Oh, sorry, my memory ia not really great and can give me false memories.
I've checked detailed and it looks the same, my bad.

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

Successfully merging this pull request may close these issues.

None yet

2 participants