Skip to content

[AI image] fix styling issues #28380

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

Merged
millerf merged 12 commits into from
Jan 18, 2023
Merged

[AI image] fix styling issues #28380

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
2786739
[not verified] Fix textarea's width
millerf Jan 16, 2023
aece91f
[not verified] Add changelog
millerf Jan 16, 2023
e173e9c
[not verified] Small refactor
millerf Jan 16, 2023
0032669
[not verified] Wrap results
millerf Jan 16, 2023
5535181
[not verified] Small refactor. Saves image in a separate function and…
millerf Jan 16, 2023
21298ee
[not verified] DOM cleanup
millerf Jan 16, 2023
02460d7
Fix small bug
millerf Jan 16, 2023
843c914
Bigger textarea
millerf Jan 16, 2023
8adae4f
Better Flex handling. Add modal
millerf Jan 17, 2023
c882bd5
Fix build and text
millerf Jan 18, 2023
0d87c56
Add button to modal
millerf Jan 18, 2023
e19b49e
Merge remote-tracking branch 'origin/trunk' into fix/ai-styling--issues
jeherve Jan 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Bigger textarea
  • Loading branch information
@millerf
millerf committed Jan 16, 2023
commit 843c914b93a1a6bc50f7e47e7f022a817ce38839
1 change: 1 addition & 0 deletions projects/plugins/jetpack/extensions/blocks/ai-image/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ export default function Edit( { attributes, setAttributes, clientId } ) {
label={ __( 'What would you like to see?', 'jetpack' ) }
placeholder={ placeholder }
onChange={ setPrompt }
rows={ 6 }
/>
<Button variant="primary" onClick={ submit }>
{ errorMessage ? __( 'Retry', 'jetpack' ) : __( 'Submit', 'jetpack' ) }
Copy link
Member

@jeherve jeherve Jan 17, 2023

Choose a reason for hiding this comment

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

This is breaking the build I'm afraid:

msgid argument is not a string literal: __(E?"Retry":"Submit","jetpack")

The building tooling doesn't seem to like the translations in a ternary like that, so I would recommend moving them out of there, maybe like so (it's not as pretty, but it does seem to build):

diff --git a/projects/plugins/jetpack/extensions/blocks/ai-image/edit.js b/projects/plugins/jetpack/extensions/blocks/ai-image/edit.js
index 3ed9e51195..f970a7d0b2 100644
--- a/projects/plugins/jetpack/extensions/blocks/ai-image/edit.js
+++ b/projects/plugins/jetpack/extensions/blocks/ai-image/edit.js
@@ -71,6 +71,9 @@ export default function Edit( { attributes, setAttributes, clientId } ) {
 	const { replaceBlock } = useDispatch( blockEditorStore );
 	const [ errorMessage, setErrorMessage ] = useState( null );
 	const [ placeholder ] = useState( getRandomItem( dalleExamplePrompts ) );
+	const errorButtonText = __( 'Retry', 'jetpack' );
+	const successButtonText = __( 'Submit', 'jetpack' );
+	const submitButtonText = errorMessage ? errorButtonText : successButtonText;
 
 	const { mediaUpload } = useSelect( select => {
 		const { getSettings } = select( blockEditorStore );
@@ -175,7 +178,7 @@ export default function Edit( { attributes, setAttributes, clientId } ) {
 								rows={ 6 }
 							/>
 							<Button variant="primary" onClick={ submit }>
-								{ errorMessage ? __( 'Retry', 'jetpack' ) : __( 'Submit', 'jetpack' ) }
+								{ submitButtonText }
 							</Button>
 						</FlexBlock>
 					</Flex>

Copy link
Contributor Author

@millerf millerf Jan 18, 2023

Choose a reason for hiding this comment

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

Funny I didn't see any issue when building 🤔

While you are here. Another question. My building keeps breaking until I do a composer update in projects/plugins/jetpack. Don't you push the new composer.lock?

Copy link
Contributor Author

@millerf millerf Jan 18, 2023

Choose a reason for hiding this comment

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

I see. You're talking about the GH actions build...

Copy link
Contributor

@artpi artpi Jan 18, 2023

Choose a reason for hiding this comment

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

See discussion in #28285

Copy link
Contributor Author

@millerf millerf Jan 18, 2023

Choose a reason for hiding this comment

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

See discussion in #28285

Thanks! I didn't see those!
So it looks like an issue with the PHP version?

Copy link
Member

@jeherve jeherve Jan 18, 2023

Choose a reason for hiding this comment

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

You're talking about the GH actions build...

Yes. We get no build errors when doing a dev build locally with jetpack build plugins/jetpack.

So it looks like an issue with the PHP version?

Yup, looks like it. You would need to update to PHP 8.0. Although in the case of this PR, you shouldn't have to run any kind of composer commands really, since you're not making any changes to composer packages. You should be able to build and update the blocks in your PRs with jetpack build plugins/jetpack and jetpack watch plugins/jetpack. I don't believe you need any other build commands at the moment. Maybe jetpack rsync to test on WordPress.com simple or WoA.

Expand Down