Dimillian / RedditOS

The product name is Curiosity, a SwiftUI Reddit client for macOS Big Sur
Apache License 2.0
3.95k stars 199 forks source link

Update layout of Post Row #25

Closed DanKorkelia closed 3 years ago

DanKorkelia commented 3 years ago

Hey! Thanks for approving my yesterday's PR. 🙏

Sending another one with small changes to post row. Hopefully nothing controversial. I can see it's a complex layout so just making a mind padding changes and the most change is to placement of image and upvote/downvote to be centred rather than at the top.

I would also like to propose to limit number of rows shown for title to provide a most consistent layout for each post.

Number number of lines logic into the enum (which can in theory be tested now).

Added a divider to give a bit more separation to each post for potentially better readability.

Attached screenshot of change.

Screenshot 2021-01-02 at 16 31 40
DanKorkelia commented 3 years ago

There is another option I can add to this to make it a bit more user friendly. Limit the number of lines to 3 for compact and to 5 for large and provide the tooltip. using .help modifier that can show full title on hover on MacOS.

.help(viewModel.post.title)

Screenshot 2021-01-03 at 15 41 00

This has a positive side effect of improving compact view also. 😁

Screenshot 2021-01-03 at 15 46 03
Dimillian commented 3 years ago

So I love those changes, all good to me.

For the title: I would keep it to unlimited lines for the normal mode, it's important to me because this is how I browse Reddit, I mostly read title and I need them in full. For the compact mode, I didn't knew about the .help modifier, it's awesome! So 3 line for the compact mode + the help sound good! You can make those changes and we'll merge :)

DanKorkelia commented 3 years ago

Cool. I will update the PR.

Can send another PR for adding tooltips / help modifier to some parts of the interface such as toolbar items.

DanKorkelia commented 3 years ago

I would suggest to keep help modifier for all layout sizes as it's a handy accessibility feature as well. It can activate only when hovering over the title as you can apply different tooltips to other elements of the view.

Dimillian commented 3 years ago

Yes of course, the help modifier can stay for all layout sizes.

DanKorkelia commented 3 years ago

Happy to merge? @Dimillian