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

"Round Profile Pictures" update to feature version 2 #494

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Wilamaxin
Copy link
Collaborator

Additional changes:

  • Added an option in settings to allow the user to choose the level of rounding percentage.

  • Fixed broken rounded edges of profile pictures in the "What's Happening?" box.

Before:

before

After:

after

Copy link
Member

@rgantzos rgantzos left a comment

Choose a reason for hiding this comment

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

You updated the features.json file for multiple features.

@Wilamaxin Wilamaxin requested a review from rgantzos June 13, 2023 23:21
Copy link
Member

@rgantzos rgantzos left a comment

Choose a reason for hiding this comment

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

Could you please also delete the old round-profile-pictures.js file?

"tags": [],
"scripts": [{ "file": "script.js", "runOn": "/*" }],
"options": [
{ "id": "Rounding Percentage", "name": "Rounding Percentage", "default": "50%" }
Copy link
Member

Choose a reason for hiding this comment

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

If we're going to add things like this, then we should also add an option validation system.

Copy link
Member

Choose a reason for hiding this comment

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

I'll make an issue for this and it should be added in the next few days.

Copy link
Member

Choose a reason for hiding this comment

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

@Wilamaxin It's been merged, something like "validation":[{ "regex": "^(100(?:\\.0{1,2})?|\\d{1,2}(?:\\.\\d{1,2})?)%$", "explanation": "Percentage must be between 0% and 100%."}] for the option would work now.

Copy link
Member

Choose a reason for hiding this comment

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

I feel like this would work better as CSS. Options create CSS variables, so something such as img[src*="/get_image/user/"] should work as a selector, and then the CSS variable could just be used for roundness.

Comment on lines +1016 to +1017
"id": "round-profile-pictures",
"version": 2
Copy link
Member

Choose a reason for hiding this comment

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

This feature was previously bugged and has a few super cool new changes with this update, so let's move this to the top of the list as well.

@rgantzos rgantzos added type: enhancement Makes an existing feature better priority: 2 The 2nd most important. bug fix labels Jun 23, 2023
@rgantzos rgantzos added status: needs review Waiting for further review. scope: feature and removed bug fix priority: 2 The 2nd most important. labels Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: feature status: needs review Waiting for further review. type: enhancement Makes an existing feature better
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants