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 user badges to entry and post templates #467

Merged
merged 5 commits into from
Jan 31, 2024
Merged

add user badges to entry and post templates #467

merged 5 commits into from
Jan 31, 2024

Conversation

e-five256
Copy link
Member

@e-five256 e-five256 commented Jan 31, 2024

this is an addition to the user badges shown on comments and user profile pages

these are useful for comments, but for users that may never post comments such as bots that only post entries or posts, there was very little clarity on the fact that it's a bot doing the posting without visiting their profile page

I figured while doing bots, might as well add mod/admin as well as I think it's useful information to be clear about, and is similar to how other sites show the information

before

image
image

after

image
image

@e-five256 e-five256 added the frontend Visual issues, improvements, bugs or other aspects relating mostly to the front end label Jan 31, 2024
nobodyatroot
nobodyatroot previously approved these changes Jan 31, 2024
@nobodyatroot
Copy link
Member

nobodyatroot commented Jan 31, 2024

One interesting thing I noticed... this L4s account is a Bot AND a Mod of these mags and the if ordering just so happens to show Mod first.... not sure if the order should be rearranged so that it checks for a Bot first?

image

image

Comment on lines 294 to 299
.user-badge {
border: var(--kbin-section-border);
padding: 0.25rem 0.5rem;
margin-left: .25rem;
border-radius: var(--kbin-rounded-edges-radius);
}
Copy link
Member

Choose a reason for hiding this comment

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

instead of copying this section we should make it global shouldn't we?

Copy link
Member Author

@e-five256 e-five256 Jan 31, 2024

Choose a reason for hiding this comment

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

I've never seen a good place for global css, if you have any suggestions let me know. CSS is divided into components, mixins, themes, and variables. components only have css that effect their single component. layout, similarly, only has things that effect the class they're named after (e.g. _options.scss starts with .options { so only effects things under that

I could instead change this to a mixin so it isn't repeated in code, but that will still increase the compiled file size by the same amount this copy paste does

Copy link
Member

Choose a reason for hiding this comment

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

it could maybe fit in the _layout.scss. If not we can make a new _global.scss which is imported from the app.scss

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't really feel it falls under the layout folder based on what's in there so I guess I'll go with the top level idea

templates/components/entry.html.twig Outdated Show resolved Hide resolved
@BentiGorlich
Copy link
Member

One interesting thing I noticed... this L4s account is a Bot AND a Mod of these mags and the if ordering just so happens to show Mod first.... not sure if the order should be rearranged so that it checks for a Bot first?

Imo it should show both, not just one of them. Currently only one of them is shown and "Mod" has precedence over "bot"

@e-five256
Copy link
Member Author

separated out the check for bot from permission level similar to op, as a user can be both a bot and mod / global mod / admin

of course a user can also likely be a mod, global mod, and admin all at the same time but as the permission set is inclusive of the previous steps doesn't seem like that needs to be bubbled up

image

(after making the bot account a global mod to test)

image
image

@BentiGorlich BentiGorlich self-requested a review January 31, 2024 14:41
@e-five256 e-five256 merged commit a722b30 into main Jan 31, 2024
7 checks passed
@e-five256 e-five256 deleted the e5/entry-badge branch January 31, 2024 15:04
andrewmoise pushed a commit to andrewmoise/mbin that referenced this pull request Feb 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend Visual issues, improvements, bugs or other aspects relating mostly to the front end
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants