Feature/add crop screen #83

ajmaurya99 · 2021-09-09T15:07:16Z

Description of the Change

This PR attempts to add a crop screen after selecting an Avatar Image. Currently the suggested image dimensions: 200 by 200 pixels.
The crop screen is skipped if the Avatar size is 200 by 200 pixels.
Select Avatar CTA is renamed to Select Avatar and Crop.
The code is updated to ES6 standards.

Screenshots

Alternate Designs

None

Benefits

Now the users can crop their Avatar as they want it to appear on the website.

Possible Drawbacks

Currently i havent used the api.CroppedImageControl and api.SiteIconControl but these can be used, in order to make the code more robust.

Verification Process

Visit the user profile screen and scroll to the Avatar section at the bottom of the page.
Try updating/adding the Avatar image and you should be able to crop the Avatar Image as per your choice.

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

#65

Changelog Entry

Antonio-Laguna

Hey @ajmaurya99 !

This is amazing and I love all the comments you've left throughout the code. Everything is very well explained and the code is easy to follow.

I've added some comments as per your request to review this which has mostly formatting things.

includes/class-simple-local-avatars.php

simple-local-avatars.dev.js

- force square aspect ratio

helen · 2021-10-01T03:14:11Z

includes/class-simple-local-avatars.php

@@ -329,8 +330,7 @@ public function avatar_settings_field( $args ) {
 	 *
 	 * @param object $profileuser User object
 	 */
-	public function edit_user_profile( $profileuser ) {
-	?>
+	public function edit_user_profile( $profileuser ) { ?>


Suggested change

public function edit_user_profile( $profileuser ) { ?>

public function edit_user_profile( $profileuser ) {

?>

@ajmaurya99 please add this suggestion by Helen.

helen

Works great! Could you possibly take a pass to reduce/remove the whitespace and other unrelated changes, as well as extraneous code comments that aren't needed for this context?

includes/class-simple-local-avatars.php

simple-local-avatars.dev.js

ajmaurya99 · 2021-10-01T11:25:49Z

@helen I have modified the PR based on your comments, Please have a look.

includes/class-simple-local-avatars.php

helen · 2021-10-18T22:17:56Z

Another general thing here is that .github/workflows/push-deploy.yml will need some changes to account for the build, along with switching from .gitattributes to .distignore and deciding whether we want to include src/** in the .org deployment. See https://github.com/10up/simple-podcasting/ for examples.

faisal-alvi

@ajmaurya99 Great work! the functionality is working as per the expectations.

Can you please work on the suggestions by Helen?
The package.json file was recently added via Introducing Package file #94. Please resolve the conflicts.
Few more suggestions below, it's negligible but good to be done now than later, please check.

includes/class-simple-local-avatars.php

src/index.js

package.json

…d with .distignore

ajmaurya99 · 2022-02-01T11:15:27Z

Another general thing here is that .github/workflows/push-deploy.yml will need some changes to account for the build, along with switching from .gitattributes to .distignore and deciding whether we want to include src/** in the .org deployment. See https://github.com/10up/simple-podcasting/ for examples.

@faisal-alvi I have replaced the .gitattributes with .distignore and added the dist folder to .gitignore.

ajmaurya99 · 2022-02-01T11:16:44Z

@ajmaurya99 Great work! the functionality is working as per the expectations.

Can you please work on the suggestions by Helen?

The package.json file was recently added via Introducing Package file #94. Please resolve the conflicts.

Few more suggestions below, it's negligible but good to be done now than later, please check.

@faisal-alvi the current feature branch is synced with the develop branch.

faisal-alvi

@ajmaurya99 thanks for the changes, added a few small suggestions, can you please check?

includes/class-simple-local-avatars.php

.distignore

includes/class-simple-local-avatars.php

faisal-alvi

@ajmaurya99 a couple of very tiny changes, everything else is good! Thanks for the efforts!

Then we are good to go 🚀.

faisal-alvi · 2022-02-04T10:48:50Z

includes/class-simple-local-avatars.php

 	private $user_id_being_edited, $avatar_upload_error, $remove_nonce, $avatar_ratings;
 	public $options;

 	/**
 	 * Set up the hooks and default values
 	 */
 	public function __construct() {
-		$this->options        = (array) get_option( 'simple_local_avatars' );
+		 $this->options       = (array) get_option( 'simple_local_avatars' );


Please remove extra space from the beginning of the variable.

faisal-alvi · 2022-02-04T10:53:54Z

includes/class-simple-local-avatars.php

@@ -329,8 +330,7 @@ public function avatar_settings_field( $args ) {
 	 *
 	 * @param object $profileuser User object
 	 */
-	public function edit_user_profile( $profileuser ) {
-	?>
+	public function edit_user_profile( $profileuser ) { ?>


@ajmaurya99 please add this suggestion by Helen.

ajmaurya99 · 2022-02-09T14:07:29Z

@faisal-alvi I have added the code indentation changes, Please have a look.

faisal-alvi

🚀

Ajay Maurya added 4 commits September 9, 2021 16:35

Initial cropping functionality added

c7b99f1

mediaUploader replaced with simple_local_avatar_frame

b50b733

labels corrected

671deb5

linting isues fixed and code updated to ES6

6fc877e

ajmaurya99 added the needs discussion label Sep 9, 2021

jeffpaul linked an issue Sep 9, 2021 that may be closed by this pull request

Add ability to crop images before setting as avatar #65

Closed

jeffpaul assigned ajmaurya99 and barneyjeffries Sep 9, 2021

Antonio-Laguna reviewed Sep 10, 2021

View reviewed changes

Ajay Maurya and others added 5 commits September 10, 2021 15:29

fix linting spacing issues

b684a1c

revert change to dev script link

dc97ba1

- update cropping logic to auto skip if image matches

6d5aef6

- force square aspect ratio

PR comments resolved for best practices

089567e

composer file synced with the develop branch

8ad9eab

ajmaurya99 requested review from Antonio-Laguna, barneyjeffries and jeffpaul September 29, 2021 11:04

ajmaurya99 removed the needs discussion label Sep 29, 2021

ajmaurya99 changed the title ~~[WIP] Feature/add crop screen~~ ]Feature/add crop screen Sep 29, 2021

ajmaurya99 changed the title ~~]Feature/add crop screen~~ Feature/add crop screen Sep 29, 2021

ajmaurya99 added the type:enhancement New feature or request. label Sep 29, 2021

ajmaurya99 marked this pull request as ready for review September 29, 2021 12:21

jeffpaul requested review from helen and removed request for jeffpaul September 29, 2021 13:23

jeffpaul added this to the 2.3.0 milestone Sep 29, 2021

helen reviewed Oct 1, 2021

View reviewed changes

helen suggested changes Oct 1, 2021

View reviewed changes

includes/class-simple-local-avatars.php Show resolved Hide resolved

simple-local-avatars.dev.js Outdated Show resolved Hide resolved

simple-local-avatars.dev.js Outdated Show resolved Hide resolved

simple-local-avatars.dev.js Outdated Show resolved Hide resolved

Ajay Maurya added 2 commits October 1, 2021 16:45

Minified file updated along with best practices

8f9e098

indentation added

dbe363a

removed package.json

0bc18ed

helen reviewed Oct 18, 2021

View reviewed changes

includes/class-simple-local-avatars.php Outdated Show resolved Hide resolved

jeffpaul requested a review from faisal-alvi January 17, 2022 22:33

faisal-alvi requested changes Jan 18, 2022

View reviewed changes

PR comments resolved around code indentation

cacc696

ajmaurya99 commented Jan 31, 2022

View reviewed changes

package.json Show resolved Hide resolved

Ajay Maurya added 5 commits February 1, 2022 13:31

webpack config updated to include the dist folder

65fa58e

build commands added in deploy pipeline and .gitattributes is replace…

4596526

…d with .distignore

sync with develop branch

e943c5e

deleted old buid files

f68f64c

deleted old buid files

c8baccf

jeffpaul requested a review from faisal-alvi February 1, 2022 14:22

faisal-alvi requested changes Feb 2, 2022

View reviewed changes

includes/class-simple-local-avatars.php Outdated Show resolved Hide resolved

.distignore Show resolved Hide resolved

includes/class-simple-local-avatars.php Show resolved Hide resolved

code indented and remove unused .distignore fie

7ef55d1

ajmaurya99 requested a review from faisal-alvi February 2, 2022 13:22

faisal-alvi requested changes Feb 4, 2022

View reviewed changes

code indentation added

d346436

jeffpaul requested a review from faisal-alvi February 9, 2022 14:28

faisal-alvi approved these changes Feb 9, 2022

View reviewed changes

jeffpaul merged commit 369fc7b into develop Feb 9, 2022

jeffpaul deleted the feature/add-crop-screen branch February 9, 2022 20:51

faisal-alvi mentioned this pull request Mar 1, 2022

Crop area does not appear sometimes. #100

Closed

1 task

faisal-alvi mentioned this pull request May 5, 2022

Fix/116 admin.js file merged into the main JS file. #118

Merged

6 tasks

dkotter mentioned this pull request May 13, 2022

Crop changes: skip cropping; allow non-square crops #128

Closed

1 task

faisal-alvi mentioned this pull request May 25, 2022

Add: Crop area options: Square, Freeform #131

Closed

1 task

faisal-alvi mentioned this pull request Sep 7, 2022

Alternate Cropping Solution #150

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/add crop screen #83

Feature/add crop screen #83

ajmaurya99 commented Sep 9, 2021 •

edited

Loading

Antonio-Laguna left a comment

helen Oct 1, 2021

faisal-alvi Feb 4, 2022 •

edited

Loading

helen left a comment

ajmaurya99 commented Oct 1, 2021

helen commented Oct 18, 2021

faisal-alvi left a comment

ajmaurya99 commented Feb 1, 2022

ajmaurya99 commented Feb 1, 2022

faisal-alvi left a comment

faisal-alvi left a comment

faisal-alvi Feb 4, 2022

faisal-alvi Feb 4, 2022 •

edited

Loading

ajmaurya99 commented Feb 9, 2022

faisal-alvi left a comment

	public function edit_user_profile( $profileuser ) { ?>
	public function edit_user_profile( $profileuser ) {
	?>

Feature/add crop screen #83

Feature/add crop screen #83

Conversation

ajmaurya99 commented Sep 9, 2021 • edited Loading

Description of the Change

Screenshots

Alternate Designs

Benefits

Possible Drawbacks

Verification Process

Checklist:

Applicable Issues

Changelog Entry

Antonio-Laguna left a comment

Choose a reason for hiding this comment

helen Oct 1, 2021

Choose a reason for hiding this comment

faisal-alvi Feb 4, 2022 • edited Loading

Choose a reason for hiding this comment

helen left a comment

Choose a reason for hiding this comment

ajmaurya99 commented Oct 1, 2021

helen commented Oct 18, 2021

faisal-alvi left a comment

Choose a reason for hiding this comment

ajmaurya99 commented Feb 1, 2022

ajmaurya99 commented Feb 1, 2022

faisal-alvi left a comment

Choose a reason for hiding this comment

faisal-alvi left a comment

Choose a reason for hiding this comment

faisal-alvi Feb 4, 2022

Choose a reason for hiding this comment

faisal-alvi Feb 4, 2022 • edited Loading

Choose a reason for hiding this comment

ajmaurya99 commented Feb 9, 2022

faisal-alvi left a comment

Choose a reason for hiding this comment

ajmaurya99 commented Sep 9, 2021 •

edited

Loading

faisal-alvi Feb 4, 2022 •

edited

Loading

faisal-alvi Feb 4, 2022 •

edited

Loading