slapperwan / gh4a

Github client for Android
Apache License 2.0
1.7k stars 221 forks source link

Make 'issues' a tab for each project. #681

Open cdysthe opened 7 years ago

cdysthe commented 7 years ago

It would be great if issues were a tab for each project. Makes navigation more efficient and consistent.

Tunous commented 7 years ago

That won't work because issues screen also has tabs for open and closed. We would need to change that to something different...

cdysthe commented 7 years ago

Couldn't there be a toggle button to include closed issues? Personally I rarely need to go through closed issues exclusively, but would at times like to see them included in the issue stream.

Tunous commented 7 years ago

Another problem I see is with actions in the toolbar. Issues screen contains here search and filter menu. If we moved issues to main screen these 2 would need to be shown with rest. That would make the main menu more cluttered and make it harder to access these actions. Probably would need to place it somewhere else or reorganize the menu.

I don't have any ideas here though...

maniac103 commented 7 years ago

Personally I like the way it's organized now. Repository tabs reflect the repo content (what do I get if I clone this repo?), and issues are not part of that.

cdysthe commented 7 years ago

Good point keeping tabs correspond with the project. Still would like access to issues wherever I am in the project. Maybe the way FastHub does it is better with a bottom bar always accessible regardless of which tab is open. For many users issues are the main reason they are using GitHub. That would be a increasing group of non developing users.

Tunous commented 7 years ago

I would be ok with that. But still there is that issue with toolbar clutter...

maniac103 commented 7 years ago

TBH, the amount of UI controls in FastHub (and the bottom bar is one of those) is one of the aspects I dislike most about it. To me, giving everything first tier placement in the UI makes it overwhelming and confusing. I'm totally open to e.g. redesigning the repo fragment to make it more obvious that the issue cell is clickable, but I have serious reservations against using a bottom bar. The issues link is among the most prominent things shown when opening a repo. For a bottom bar to be useful for non-developing users, said user would have to switch to another tab first (which he presumably wouldn't be interested in) and then not find back - is that really a likely scenario?

jancborchardt commented 7 years ago

From a UX point of view, the current way to get to issues is very hard to discover.

You do expect it in the tab bar just like it is on Github.com - the numbers being clickable is not obvious and they are also not available from every view. The tab bar should reflect the tab bar from the web interface.

For open/closed I agree with things said above: Open is definitely more important, "show closed issues" could be a toggle in the 3-dot menu even.

cdysthe commented 7 years ago

I have to agree with @jancborchardt on this. Took me a week to think of clicking the issue counter. I think the way it's done on FastHub (which I in other aspects like less) is a good way to do it using an auto hide bottom toolbar. Another option is to design the clickable items like issues to look more like a toolbar with buttons.

Tunous commented 7 years ago

Another option is to design the clickable items like issues to look more like a toolbar with buttons.

I think we should experiment with that. Some mockup or inspiration images would be helpful.

smichel17 commented 7 years ago

I think the road forward involves merging watch/star/fork functionality with the w/s/f display. I'd do it like FastHub: number underneath the icon, which is colorized if you have w/s/f'd; tap takes the action; long-press to views the list. Of course, FastHub makes the mistake of putting this all in the chrome; we should put it where issues/stargazers/forks/pulls currently are, and move issues & pulls to their own tabs as this issue suggests. Reference image:

image

I also suggest adding an additional item at the end: a drop-down arrow, collapsed by default, that expands to show the Other information section. This would make #707 unnecessary, since there's no content below the readme. Or we could keep #707 and show some other information (activity feed, so we don't have so many tabs?) down there.

Tunous commented 7 years ago

tap takes the action; long-press to views the list

Long press is hard to discover (I didn't know you could do that on FastHub) Perhaps clicking on these could show menu with action to select (e.g. watch or view watchers)

For the tabs we could go with:

About | Issues (84) | Pull Requests (9) | Activity

Which would be much like tabs on GitHub. (Points for interface being familiar to users.)

Files, commits, branches could go to other information screen either as separate options or one that starts new screen with these as tabs. We would lose fast access to these, but I think it's a compromise we can accept. (I use files a lot so it's a huge compromise for me)

Or files could be made as similar expandable card to readme with just top level files where clicking on any directory would open new screen with just the navigation. Again this would make it similar to how it's done on GitHub and we wouldn't lose fast access to file browser.

Readme could be collapsed by default so users can easily see other information.

smichel17 commented 7 years ago

Long press is hard to discover (I didn't know you could do that on FastHub) Perhaps clicking on these could show menu with action to select (e.g. watch or view watchers)

Discoverability is bad but functionality is very nice. Maybe we could show a toast the first few times the user taps, instead? "Long press to see watchers". Or if we don't want to track how many times a user has tapped, we could show the toast always, when they un-watch. Reasoning: if a user taps expecting to see the list, then they'll immediately un-watch, and then they'll see the notification. At the same time, un-watch isn't a very common action, so we won't be spamming toasts.

Files, commits, branches could go to other information screen [...] Or files could be made as similar expandable card

These all seem a little clunky to me, and I can't think of a better way right now. I'll sleep on it.

Note: The current interface for changing branches is confusing to me, because I associate that icon with forking. I tapped on the repo name (since it says "master" right under) several times, before I tried the right icon. Whatever we decide, I'd appreciate if we change that part :)

Readme could be collapsed by default so users can easily see other information.

IMO that information belongs above the readme but collapsed by default the way I suggested in my comment above. Something like this (obviously with a couple styling changes):

repo_collapsed repo_expanded

maniac103 commented 7 years ago

Why not display it in rows? Each row could have an icon, text (e.g. '123 stars') and a button ('Show'). Tapping the icon (or the whole row?) would carry out the toggle action. IMHO this would both look better and be easier understandable than the Fasthub-like approach.

Issues and PRs would fit in well there, which IMHO is a good thing, as I don't like the idea of burying away the repo content access.

Tunous commented 7 years ago

@maniac103 Sounds good to me. I can hack a test implementation tomorrow unless someone beats me to it 😏

smichel17 commented 7 years ago

Why not display it in rows?

The only reason would be to conserve space -- I don't think it's a good idea to push the readme so far down that it's not even visible when the repo is first loaded. But, that's easily mitigated by allowing the link section to collapse, so +1

Tunous commented 7 years ago

screenshot_1504968235 screenshot_1504969299 screenshot_1504969329

maniac103 commented 7 years ago

Yeah, that goes into the right direction :-) I find the buttons a little stark, though. What I thought of would be mostly like the 3rd version, but with green text on white background (like the 'reply' button in reviews), a little smaller font for the text and icons to the left of the text in our usual color (grey). The watcher/star icons could become green if that repo is watched/starred.

Could you put that code into some branch, please?

Tunous commented 7 years ago

Pushed here: https://github.com/slapperwan/gh4a/tree/repository-stats-views

maniac103 commented 7 years ago

This is what I thought of: screenshot_1505039881

(icons should match in color and size in the final solution, obviously ;-) ) For watch/star, clicking the icon would toggle the state.

slapperwan commented 7 years ago

How about without Show/View label, just tap on label will do. And I agree on watch/star toggle.

maniac103 commented 7 years ago

The trigger of this change was clickability not being obvious enough, and I'm not sure whether it's obvious enough without this label?

Tunous commented 7 years ago

Perhaps add 2 buttons for top 3 rows. Show | Unwatch, etc. Otherwise viewing watchers would become obvious but toggling watch status would be hard to discover.

Also clicking on the row itself should just work same as clicking on show button. (I think it's possible to forward click highlight to that button right?)

And all the buttons should say the same thing (either show or view, not sure which is better)

smichel17 commented 7 years ago

I'm in favor of @maniac103's latest mockup. I agree functionality to star may not be obvious, though.

maniac103 commented 7 years ago

Perhaps add 2 buttons for top 3 rows. Show | Unwatch, etc. Otherwise viewing watchers would become obvious but toggling watch status would be hard to discover.

That's true, but I fear we'll get screen space issues with smaller phones when adding two buttons.

Maybe we could use different icon colors (grey vs. black/green [unselected/selected]) to indicate clickability?

Tunous commented 7 years ago

Should be able to wrap the number with text

maniac103 commented 7 years ago

Another idea: Use icons (arrow pointing rightwards) instead of the 'show' text.

slapperwan commented 7 years ago

Like gh4a version 1.0? :D

maniac103 commented 7 years ago

I'd do it a little less obtrusive, but yes ;-)

smichel17 commented 7 years ago

I prefer the link text to arrows; it feels more material. That said, a similar styling behind the icons in your mockup might help reveal them to be buttons.

Tunous commented 7 years ago

Another idea: Use icons (arrow pointing rightwards) instead of the 'show' text.

I'd like to see that.

maniac103 commented 7 years ago

Another idea: Use icons (arrow pointing rightwards) instead of the 'show' text.

I'd like to see that.

Something like this. screenshot_1505138807

(not sure whether the dividers are helpful or not)

Tunous commented 7 years ago

Yes, I like that. Dividers make it easier to read. Also, could align text with toolbar title.

Just it'll be hard to discover that the icons are clickable...

Tunous commented 7 years ago

Icon recommendations:

For forks: fork icon

For branch action in toolbar (fits better to the rest of icons): branch icon

maniac103 commented 7 years ago

Your links seem broken?

Tunous commented 7 years ago

Fixed

maniac103 commented 7 years ago

Just it'll be hard to discover that the icons are clickable...

Could use color coding for that (we'd need different icons for active vs. inactive in that case) screenshot_1505140665

Also, looking at the image, I wonder whether language shouldn't get a row as well. If we're going to restyle like that, the user activity also needs the same treatment.

cdysthe commented 7 years ago

I really like @maniac103 's latest above. Very clear and easy to understand.

Tunous commented 7 years ago

I wonder whether language shouldn't get a row as well

Where clicking on it would lead to?

the user activity also needs the same treatment

What do you mean? EDIT: Nvm you mean UserActivity. Agreed.


Make the text aligned with title (72dp from left I believe) right now it's too close to icons.

maniac103 commented 7 years ago

I wonder whether language shouldn't get a row as well

Where clicking on it would lead to?

Nowhere, which is why it wouldn't get the arrow.

the user activity also needs the same treatment

What do you mean?

Replacing the cells for followers/following/repos/gists by rows.

Make the text aligned with title (72dp from left I believe) right now it's too close to icons.

That's details - the image above was generated from an ugly hack anyway :-)

Tunous commented 7 years ago

What about applying similar styling to the other information card? (Dividers, arrows icon)

maniac103 commented 7 years ago

What about applying similar styling to the other information card? (Dividers, arrows icon)

Worth a try, but my gut feeling is that it'll look 'too much', because there's nothing else in this card.

jruesga commented 7 years ago

I'm sure you don't want to implement the row arrow thing ;)

https://developer.android.com/design/patterns/pure-android.html

maniac103 commented 7 years ago

Pushed my implementation with proper icons, but without star/watch support (for now) here: https://github.com/slapperwan/gh4a/commits/repository-stats-views

I'm sure you don't want to implement the row arrow thing ;) https://developer.android.com/design/patterns/pure-android.html

Damn, Jorge, don't start this discussion :-P Seriously though, our case may be a little less obvious than their prefs example because of the card and the mixture of clickable and non-clickable items?

jancborchardt commented 7 years ago

@maniac103 the tap targets in your mockup seem really small, also vs the big amount of space they use. The one row of icons seemed better and mainly needs to have the clickability affordance added. Maybe just via circles around them and the green color?

Tunous commented 7 years ago

the tap targets in your mockup seem really small

How so? It's much bigger tap area than before (whole row vs circle)

jancborchardt commented 7 years ago

@Tunous the height of the row seems insufficient. Also the visual appeal is way less than the icons before – and the arrow on the right is very far away from the actual text.

maniac103 commented 7 years ago

the height of the row seems insufficient.

That's something we need to fiddle with to find the balance between the needed screen space and touch target size. I've used 36dp at the moment; maybe 40dp would be a good compromise.

I've found, though, that it's kind hard to judge touch target sizes from screenshots alone, so feel free to give the code a try.

Also the visual appeal is way less than the icons before

What do you mean by 'the icons before'?

and the arrow on the right is very far away from the actual text.

It's right-aligned. Putting it next to the text would look ugly due to them not being aligned.

smichel17 commented 7 years ago

Hey, I just realized we missed something totally obvious: we could put the star/watch/ whatever action on the next screen. There's absolutely no menu items there at all so we can have a full text action, "Star"/"Fork"/"Watch".

I think I'd do this in addition to making the icons tap-able for quick access. This solves the discovery issue without sacrificing any convenience.


I like the arrows way more than I thought I would, better than the text, even. That's because of the dividers, which I was not imagining.

However, the dividers strongly suggest that the entire thing is one touch target, so I would not make the icon separately clickable in that case.

If you decide to go the action text route, I would make tapping the word "Stars"/ "watchers" / etc be part of the icon touch target, not the "show" target, and I'd leave the whitespace in between dead. The green text is enough to get people to click it.


I think at this point deciding between the two options needs to be done by playing around with the prototype; we're beyond the point where theorizing about usability is really helpful.

jancborchardt commented 7 years ago

@maniac103 I mean that the proposal further up in the thread with the simple one row of 5 icons next to each other and text below looks much nicer than a bland list.

Additionally, it makes the beginning of the Readme visible which gives a direct peek at the contents of the project.