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

Add FeaturedPosts support #873

Merged
merged 1 commit into from
Dec 14, 2022
Merged

Conversation

makotech222
Copy link
Contributor

See LemmyNet/lemmy#2585

Adds a new 'Pin' icon to pin to Local view

image

Marking as draft, as its still a bit unfinished.

Also, soliciting feedback on what the Icon should look like. I'm not a fan of both 'pin to community' and 'pin to local' having the same icon. Not sure what other icons are available currently.

@makotech222
Copy link
Contributor Author

One last design issue I have currently is when to display the 'pin' icon when displaying the post. We can view a Post from multiple places throughout the app, Home, Community, Search, etc. Currently, if a post is 'Featured in Community', it will show the pin icon, even when viewing from outside the community.

Should we differentiate when viewing a post from multiple places? Should I just display the 'pin' icon if its either community or local featured?

image

@makotech222
Copy link
Contributor Author

@dessalines tagging you here, sorry if its doubled up. for the question above.

@makotech222
Copy link
Contributor Author

@Nutomic tagging, any thoughts on the two issues above?

@Nutomic
Copy link
Member

Nutomic commented Dec 9, 2022

It should definitely not use the same icon, thats very confusing. If you cant find a fitting icon, why not put a text button like the one currently used for "remove"?

@dessalines
Copy link
Member

Hrm... it doesn't matter to me, but probably not text. Different color pin icons should work, text-primary and text-secondary.

@makotech222
Copy link
Contributor Author

Okay ill try different color pins. How about the other issue? On showing pinned icon?

@dessalines
Copy link
Member

It pry makes sense to show the community pin, even if you're viewing it from the front page. No need to create rules around when to show it IMO.

@makotech222
Copy link
Contributor Author

makotech222 commented Dec 9, 2022

Okay, here's text-primary for community feature, text-secondary for local feature:

image

For the buttons, they are all 'text-muted', so maybe a different color might not be visually pleasing:

image

I could add text to the buttons:

image

Otherwise, we need a new icon. I asked in the lemmy chat, but couldn't get a good list of icons that are actually imported. What icons are available to use?

@dessalines
Copy link
Member

Looks fine to me, but maybe they should be hidden behind the 3-dot menu. Up to you tho.

@makotech222
Copy link
Contributor Author

makotech222 commented Dec 11, 2022

The three-dot button is already expanded, and the pin icons only show when expanded.

Also, which one looks good? Text with icon?

@dessalines
Copy link
Member

Yeah text with icon is fine.

@makotech222 makotech222 force-pushed the featuredposts branch 2 times, most recently from 0875adb to 84568cb Compare December 11, 2022 21:16
@makotech222 makotech222 marked this pull request as ready for review December 11, 2022 21:17
@makotech222
Copy link
Contributor Author

Okay should be ready to go then! Thanks!

Copy link
Member

@dessalines dessalines left a comment

Choose a reason for hiding this comment

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

Looks good, thx!

@dessalines dessalines merged commit 003b177 into LemmyNet:main Dec 14, 2022
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.

3 participants