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

#597 add border behind pfp #681

Merged
merged 4 commits into from
Feb 4, 2025
Merged

Conversation

kuba-04
Copy link
Contributor

@kuba-04 kuba-04 commented Jan 28, 2025

fixes #597

@jb55
Copy link
Contributor

jb55 commented Jan 28, 2025

I think it would make more sense to add this as an option on ProfilePic. We wouldn't always want a border. We should also just put a full circle behind it instead of just a border, because just a stroke seems to be creating artifacts around the border of the image

@kernelkind
Copy link
Member

Can also experiment with Painter::round_to_pixel

@kuba-04
Copy link
Contributor Author

kuba-04 commented Jan 29, 2025

Sure!

  1. I just added that full circle behind. We may avoid adding inner circle if not image is available. Please let me know which one you like more:

A) version with inner circle:

image

B) version without inner circle:

image

  1. As for the option for ProfilePic struct, I have some problem with this. Maybe I misunderstood, but if I add it to the struct, then there will be many places to choose on that option. It actually showed me how many times we have to create a new ProfilePic, do we need so many of them? I will double check that. But for this issue basically my question is: where do we want to decide on that border to exist or not? EDIT: To be more precise with 'where', we can decide in this match:

image

or you have plans to customize this border/circle with some feature which will load the color or something from the profile info..

btw, these are the places of creating new ProfilePic:
image

@kuba-04
Copy link
Contributor Author

kuba-04 commented Jan 29, 2025

Just seen your comment:

Can also experiment with Painter::round_to_pixel

tried now, so far it didn't help. I will try again later

@jb55
Copy link
Contributor

jb55 commented Jan 29, 2025 via email

@kuba-04
Copy link
Contributor Author

kuba-04 commented Jan 30, 2025

you set it via a method on ProfilePic. something like
ui.add(ProfilePic::new().border(2.0))

thanks, I am here to learn from you guys.

Added, but not sure if the size is good in all places

@jb55
Copy link
Contributor

jb55 commented Jan 31, 2025 via email

@jb55 jb55 self-requested a review February 4, 2025 02:30
@jb55 jb55 merged commit 635c977 into damus-io:master Feb 4, 2025
@jb55
Copy link
Contributor

jb55 commented Feb 4, 2025

thanks! sorry for the delay, I added a commit on top of this to make it look bit cleaner:

9dd33d5

@kuba-04
Copy link
Contributor Author

kuba-04 commented Feb 4, 2025

nice! if you don't mind I am gonna pick up more issues for notedeck as I am using the app regularly.

Copy link
Contributor

jb55 commented Feb 4, 2025

great!

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.

Border behind PFP
3 participants