Display sort name as subtitle and make sort button use generic sort icon

[tom-james-watson]

Pull Request Description

Displays the currently-selected sort option as the subtitle on the community page. It also updates the sort button to use a generic sort icon instead of the sort-specific icons that are currently used.

This is a follow up to #616 where some discussion led to this current idea being hashed out.

Issue Being Fixed

The icons used for the different kinds of sort are not all intuitive. It's not always clear what they mean and so it's sometimes hard to tell what you are currently looking at unless you are deeply familiar with the app.

Issue Number: N/A

Screenshots / Recordings

Checklist

• Did you update CHANGELOG.md?
• Did you use localized strings where applicable?
• Did you add semanticLabels where applicable for accessibility?

[micahmo]

LGTM, approved! Thanks for iterating on this and coming up with something we all like!

I would be interested to hear feedback from @ajsosa and @machinaeZER0 before merging.

Some more random musings that popped into my head...

• Is the vertical height of the heading too much now? Should it match the height of the other AppBar icons?
• Would it be cool to show the legacy sort icon in the heading for those who have memorized those and as a bit of a distinguisher from other apps, or is that too much clutter in the heading?
• Should we make the same change to the post view?
  • I don't think the post view has any heading right now. The heading could be the post title or the community name or something else.
• Should we make any changes to the sort option in the FAB? Right now in community view it shows the current sort icon and in post view it shows the generic icon, so it's inconsistent either way.

Anyway, these points are all for discussion / future work. Thanks again!

[machinaeZER0]

I would definitely update the FAB and post view for consistency! Defer to others whether that is necessary here, or would be addressed in a separate PR :)

And agreed, thanks for working on this!

[micahmo]

@machinaeZER0 How do you feel about the vertical height of the heading now? Does it look funny extending past where the buttons go?

[machinaeZER0]

Sorry, is it possible to see a before/after screenshot? Want to make sure I'm looking at the specific change you mean

[micahmo]

I know I'm majorly nitpicking at this point 😆 Also pardon the awful lines.

[machinaeZER0]

Ahh, thank you! I personally don't mind how this is laid out :) if it breaks over two lines like this then I think the icons being vertically centered is the expected approach (unless you're saying they're not quite centered?)

What was your thinking in terms of an alternative?

Also, I wouldn't be opposed to some sort of implementation that includes icons as well (to your other point), but personally I am good with this overhaul that does not feature them. Would be happy to look at a mockup if you felt strongly though!

[micahmo]

if it breaks over two lines like this then I think the icons being vertically centered is the expected approach (unless you're saying they're not quite centered?)

Sorry I still wasn't clear! I meant should the heading be shrunk to fit so it doesn't overflow. But looking at other apps, they overflow too (maybe to a lesser extent), so I'll drop it. 😆

[machinaeZER0]

We could always revisit shrinking the title - I kinda like the size it is, but I don't feel super strongly about it!

[micahmo]

I'll trust your judgement on the UI! @tom-james-watson want to resolve the conflict and I'll merge?

[tom-james-watson]

For reference, this is the default size of the title in the ListTitle widget.

Personally I prefer the larger text. It also keeps it consistent with the title sizing on other pages.

[tom-james-watson]

Fixed the conflict 👍