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 role badges on public #24729

Closed
wants to merge 3 commits into from

Conversation

cdp1337
Copy link

@cdp1337 cdp1337 commented Apr 30, 2023

Fixes #19922

New account component for account badges and implemented on a few places on the public UI. Supports the public toggle and color definition from admin.

New account component for account badges and implemented on a few places on the public UI.
Supports the public toggle and color definition from admin.
New new strings, but removed 'Group' as the account badge is using the text entered by the admin
@ClearlyClaire ClearlyClaire changed the title Implement #20031 - Role UI on public Add role badges on public May 2, 2023
@ClearlyClaire
Copy link
Contributor

Hi! Thank you for your contribution!

When making UI changes, please try to provide screenshots with your PRs!

The check-i18n failure is because your commit 9ecee34 added a newline at the end of the defaultMessages.json file.

Also, just for your information, there is another PR trying to solve this with a different design (#21393), although we are not quite sure what design to go with for now, but the main thing that looks off with your PR is how the badges are rendered in posts. It would probably be ok leaving them out of this view.

@cdp1337
Copy link
Author

cdp1337 commented May 3, 2023

Yeah, I noticed the newline issue, but hadn't had a chance to commit the fix yet.

Screenshot from 2023-04-29 23-59-20
Screenshot from 2023-04-29 23-59-44
Screenshot from 2023-04-30 00-00-26

Also screenshots! I posted them in #20031 but forgot to do so here, (I tend to forget that merge requests and issues are duplicates of each other for some weird reason...)

I didn't change the UI of the existing badges, just expanded support for roles on them, (other than adding the tags for admin-defined colours).

@ClearlyClaire
Copy link
Contributor

I didn't change the UI of the existing badges, just expanded support for roles on them, (other than adding the tags for admin-defined colours).

I meant the addition of badges in posts (your changes to app/javascript/mastodon/features/status/components/detailed_status.jsx, the second screenshot above) is the part that looks especially off and is probably better left off.

Otherwise, it looks good to me, but we'll wait for feedback from our designer.

@github-actions
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

@VityaSchel
Copy link

you are cool

@ClearlyClaire
Copy link
Contributor

Hi! Thank you for your contribution, but following input from our designer, it has been implemented as #25649 and #26281

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Staff badges missing on new instance
3 participants