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.

Sign up for GitHub

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

Jump to bottom

LPS-125413 Use new ImageSelector component in image-selector tag and remove old one #98928

Closed
wants to merge 8 commits into from
Closed

LPS-125413 Use new ImageSelector component in image-selector tag and remove old one #98928

wants to merge 8 commits into from

Conversation

liferay-continuous-integration
Copy link
Collaborator

Forwarded from: liferay-content-management#1494 (Took 2 ci:forward attempts in 6 days 7 hours 21 minutes)

@carloslancha
@liferay-lima

Original pull request comment:
Following up on liferay-content-management#1484

https://issues.liferay.com/browse/LPS-125413

In this pr we're removing the old image-selector AUI component in favor of the new React one.

We're adding some missing markup in the React component and removing no needed one for SSR in the JSP

Made also some refactor and renaming in the React Components.

Deprecate draggableImage in favor of new imageCropDirection attribute in ImageSelector tag to improve naming.

imageselector

Test Plan 1:

  • Go to Content & Data / Blogs
  • Create a new Blog
  • Select an image bigger than the container
  • Click and move vertically the picture to crop it in the desired position
  • Publish
  • Edit the blog
  • Check that the image has been cropped correctly

Test Plan 2:

  • Go to Content & Data / Blogs
  • Edit an existing Blog with an image
  • Check that you can't move the image since it's already cropped
  • Remove the image and repeat Test plan 1 from point 3.

✔️ ci:test:stable - 9 out of 9 jobs passed

✔️ ci:test:relevant - 23 out of 23 jobs passed in 1 hour 20 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 14013e30ddc196a32bec7e7d3ad31bc64eff58be

Upstream Comparison:

Branch GIT ID: 3a1328763cf4d2349d46f0a0674429df321a469f
Jenkins Build URL: Acceptance Upstream DXP (master) #1560

ci:test:stable - 9 out of 9 jobs PASSED
9 Successful Jobs:
ci:test:relevant - 23 out of 23 jobs PASSED
23 Successful Jobs:
For more details click here.

✔️ ci:test:sf - 1 out of 1 jobs passed in 47 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 1110b647c2539d8b3ba1fff788333bb9e010f4e9

Sender Branch:

Branch Name: LPS-125413-migrate-image-selector.2
Branch GIT ID: 5c0eaea602397ca54c3e71570e129e3a4f33e102

1 out of 1jobs PASSED
1 Successful Jobs:
For more details click here.

Closed
@liferay-continuous-integration
Copy link
Collaborator Author

To conserve resources, the PR Tester does not automatically run for forwarded pull requests.

@brianchandotcom
Copy link
Owner

@carloslancha why is it

	<div className="change-image-controls">
		<ClayButtonWithIcon
			className="browse-image"

Shouldn't it be <div class vs className?

				<clay:button
					cssClass="ml-1"

That part is right. We usually call it "cssClass" on the Java side to distinguish it from "String className = Object.class.getName()"

@wincent @jbalsas

Please resend. Thx.

@brianchandotcom
Copy link
Owner

Merged. Thank you.
View total diff: eed34fb...1ee911c

@brianchandotcom
Copy link
Owner

@carloslancha @wincent @jbalsas

Nevermind, I just did:

git grep "<div className"

and see it everywhere.. must be some weird React thing.

@carloslancha
Copy link

Yup @brianchandotcom in React element classes are set with className because - AFAIK - class is a reserved word

@wincent
Copy link

wincent commented Feb 19, 2021

because - AFAIK - class is a reserved word

Originally it was because React mostly used names from the DOM API (where it is className) as opposed to HTML attribute names (eg. class).

There has long been talk of moving towards class though, but as of right now, the proposal to do so is shelved.

@liferay-continuous-integration liferay-continuous-integration deleted the ci-forward-LPS-125413-migrate-image-selector.2-pr-1494-sender-carloslancha-ts-1613669086373 branch February 26, 2021 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
ci:test:relevant - success ci:test:sf - success ci:test:stable - success
Projects
None yet
Milestone
No milestone
5 participants
@liferay-continuous-integration @brianchandotcom @carloslancha @wincent @nikki-pru