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

Ability to set default avatar #96

Merged
merged 4 commits into from Apr 27, 2022
Merged

Conversation

mehulkaklotar
Copy link
Contributor

Description of the Change

Alternate Designs

Benefits

It will allow admins to use custom default image avatars for users instead of using gravatar defaults.

Possible Drawbacks

Verification Process

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

Changelog Entry

@mehulkaklotar mehulkaklotar self-assigned this Jan 6, 2022
@mehulkaklotar mehulkaklotar linked an issue Jan 6, 2022 that may be closed by this pull request
2 tasks
@faisal-alvi
Copy link
Member

Please note I have merged develop in the feature/default-avatar to resolve conflicts.

@mehulkaklotar mehulkaklotar marked this pull request as ready for review April 18, 2022 11:14
@mehulkaklotar
Copy link
Contributor Author

Hi @faisal-alvi
can you run a quick look on the changes if they can pass or not? I have tested the default avatar in local but I would like to be tested more.
Thanks.

Copy link
Member

@faisal-alvi faisal-alvi left a comment

Choose a reason for hiding this comment

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

@mehulkaklotar thanks for the efforts. I have checked and the avatar seems to be broken after being selected. Not sure if it's an issue from my side.

sla-96
image link

Also, I have added a few suggestions/comments below, please have a look.

assets/js/admin.js Outdated Show resolved Hide resolved
assets/js/admin.js Outdated Show resolved Hide resolved
assets/js/admin.js Outdated Show resolved Hide resolved
includes/class-simple-local-avatars.php Outdated Show resolved Hide resolved
includes/class-simple-local-avatars.php Outdated Show resolved Hide resolved
includes/class-simple-local-avatars.php Outdated Show resolved Hide resolved
includes/class-simple-local-avatars.php Outdated Show resolved Hide resolved
includes/class-simple-local-avatars.php Show resolved Hide resolved
includes/class-simple-local-avatars.php Outdated Show resolved Hide resolved
assets/js/admin.js Outdated Show resolved Hide resolved
@jeffpaul jeffpaul added this to the 2.4.0 milestone Apr 20, 2022
@mehulkaklotar
Copy link
Contributor Author

Hi @faisal-alvi
Thank you for the great suggestions. I have updated the PR according to your changes.

About the issue of not appearing the default avatar for you. May be you need to regenerate the assets to see the changes. Please try that and see if that resolves the issue for you.

Copy link
Member

@faisal-alvi faisal-alvi left a comment

Choose a reason for hiding this comment

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

@mehulkaklotar thank you for your great work! ❤️ 🚀

@faisal-alvi faisal-alvi merged commit 72fd869 into develop Apr 27, 2022
@faisal-alvi faisal-alvi deleted the feature/default-avatar branch April 27, 2022 06:38
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.

Ability to set default avatar
3 participants