fix: fix element thumbnail visual issues #3329

neatbyte-vnobis · 2023-05-24T16:14:08Z

Changes

Fix issue "Block Thumbnail visual issues"

How Has This Been Tested?

Manual

Documentation

None

SvenAlHamad · 2023-05-26T13:07:53Z

Hey @neatbyte-vnobis - I don't think we want to add the font definitions into the global.scss file...several reasons for that.

Can we explore another potential approach. Instead of generating thumbnails, can we render the block content directly inside the preview container. Similarly how the page preview is rendered in the drawer, would that be possbile?
The benefit of this approach is that if a user updates the theme file, say they change the primary color, currently the image previews don't get updated with that new information. Rendering the preview directly would reflect this change immediately.

adrians5j · 2023-05-29T12:13:34Z

Did a quick POC for @neatbyte-vnobis here, let's see how far we get.

adrians5j

Starting the review with a couple of quick comments.

Additional questions below.

1. Image issue

Adding an image works:

But when saving, I get this error:

Wondering if simply pulling in all changes from next would resolve this issue? 🤔

2. Still using image?

Also, shouldn't this modal also be rendering the block, instead of displaying its sshot? Did I miss smt here? 🤔

packages/app-page-builder/src/pageEditor/config/blockEditing/BlockPreview.tsx

packages/app-page-builder/src/pageEditor/config/blockEditing/EditBlockDialog.tsx

packages/app-page-builder/src/editor/plugins/elementSettings/save/SaveDialog/domToImage.ts

neatbyte-ivelychko · 2023-07-11T08:05:09Z

Additional questions below.

1. Image issue

About this one
I've double checked it locally and on the admin app, and seems like it works as expected (saving image and not throwning an error)

But I want to say that I was having the same issue as this one when I was working on Dynamic Images (Dynamic Pages).

neatbyte-ivelychko · 2023-07-11T08:07:08Z

2. Still using image?

Regarding this question, I've double checked and I want to say that we do use Component Preview instead of Image Preview. Maybe you need to try to re-deploy admin, because for me it works as expected

packages/app-page-builder/src/pageEditor/config/blockEditing/EditBlockDialog.tsx

packages/app-page-builder/src/editor/plugins/elementSettings/save/SaveDialog/domToImage.ts

packages/app-page-builder/src/blockEditor/config/eventActions/saveBlock/getPreviewImage.ts

adrians5j · 2023-07-11T11:02:19Z

packages/app-page-builder/src/editor/plugins/elementSettings/save/SaveAction.tsx

-                ...meta,
-                private: true
-            }
+        formData.preview = {


Do we need this on the backend? 🤔

I think not, maybe I'm wrong?

This question still remains.

Maybe we don't. But I need to spend a little bit more time on this question

@adrians5j I have removed preview form the API, as we no longer need it

packages/app-page-builder/src/admin/components/EditorPluginsLoader.tsx

adrians5j · 2023-07-18T07:42:30Z

packages/app-page-builder/src/pageEditor/config/blockEditing/BlockPreview.tsx

@@ -46,6 +45,9 @@ const BlockPreview: React.FC<BlockPreviewProps> = props => {
                {onDelete && (
                    <Styled.DeleteBlock>
                        <ConfirmationDialog
+                            // We need to have this z-index because without it Delete Block Dialog will be rendered below.


Please recheck this grammar. I think the dot after the rendered below is a mistake?

Sorry for the nitpick, but yeah, we're OSS, and companies tend to check our code sometimes before making their decision. I know this is just a drop in the sea, but yeah, gotta start somewhere. :)

adrians5j · 2023-07-18T07:42:54Z

packages/app-page-builder/src/pageEditor/config/blockEditing/EditBlockDialog.tsx

-        width: 600,
-        minWidth: 600
+const StyledDialog = styled(Dialog)`
+    // We need to have this z-index because without it Edit Block Dialog will be rendered below.


Same comment re grammar.

adrians5j · 2023-07-18T07:44:54Z

packages/app-page-builder/src/editor/plugins/elementSettings/save/SaveAction.tsx

-                ...meta,
-                private: true
-            }
+        formData.preview = {


This question still remains.

adrians5j

Close!

Left minor comment and there's a question that's still unanswered.

Tested it manually, looks good. Glad we've managed to deleted some of those dom-to-image files.

One final thing to check on the UI side:

This list is rendered a bit weirdly. Can we double check that before merging?

Thanks!

neatbyte-ivelychko · 2023-07-18T08:38:03Z

One final thing to check on the UI side:
This list is rendered a bit weirdly. Can we double check that before merging?

Sure, will check it

adrians5j

Looks good. We just need to revisit this question.

neatbyte-ivelychko · 2023-07-26T08:03:58Z

@adrians5j l've checked whether we need to save preview on the back-end, and it looks like we don't, because we are rendering element itself instead of image. I will remove preview from the api.

webiny-js/packages/app-page-builder/src/editor/plugins/elementSettings/save/SaveAction.tsx

Line 66 in f5370b5

formData.preview = {

neatbyte-ivelychko · 2023-07-26T11:13:50Z

@adrians5j I've removed preview from the api

neatbyte-vnobis · 2023-08-04T11:40:07Z

/cypress

github-actions · 2023-08-04T11:40:21Z

Cypress E2E tests have been initiated (for more information, click here). ✨

github-actions · 2023-08-04T11:43:50Z

Cypress E2E tests have been initiated (for more information, click here). ✨

github-actions · 2023-08-04T11:43:54Z

Cypress E2E tests have been initiated (for more information, click here). ✨

adrians5j · 2023-08-07T12:33:24Z

packages/app-page-builder/src/types.ts

@@ -632,8 +632,7 @@ export type PbEditorBlockPlugin = Plugin & {
    blockCategory: string;
    tags: string[];
    create(): PbEditorElement;
-    image?: Partial<File>;
-    preview?(): ReactElement;
+    preview(): ReactElement;


Do we need this still here? 🤔

…3329)

neatbyte-vnobis added 2 commits May 24, 2023 15:51

fix: requested changes

c53013e

fix: fix element controls visible on preview

ee17355

neatbyte-vnobis requested review from adrians5j and Pavel910 May 24, 2023 16:14

neatbyte-vnobis force-pushed the neatbyte/dom-to-image-element branch from e6b4680 to ee17355 Compare June 14, 2023 12:15

neatbyte-vnobis and others added 4 commits June 14, 2023 15:15

Merge branch 'next' into neatbyte/dom-to-image-element

6a542a8

fix: implemented suggested changes

0868ed2

Merge branch 'next' into neatbyte/dom-to-image-element

4b8352e

fix: fixed type issue

3cf01e9

adrians5j requested changes Jul 10, 2023

View reviewed changes

neatbyte-vnobis added 2 commits July 11, 2023 12:01

Merge branch 'next' into neatbyte/dom-to-image-element

653b8e4

fix: moved zIndex style

f5370b5

adrians5j reviewed Jul 11, 2023

View reviewed changes

packages/app-page-builder/src/pageEditor/config/blockEditing/EditBlockDialog.tsx Outdated Show resolved Hide resolved

packages/app-page-builder/src/editor/plugins/elementSettings/save/SaveDialog/domToImage.ts Outdated Show resolved Hide resolved

adrians5j requested changes Jul 11, 2023

View reviewed changes

fix: deleted domToImage function

2a233a9

adrians5j reviewed Jul 18, 2023

View reviewed changes

adrians5j requested changes Jul 18, 2023

View reviewed changes

neatbyte-vnobis added 2 commits July 18, 2023 13:09

fix: fixed comment and also fixed styles for all blocks component

9cd8169

fix: fixed lint errors

bf68675

adrians5j requested changes Jul 25, 2023

View reviewed changes

fix: remove preview form the api

5a49afc

neatbyte-vnobis force-pushed the neatbyte/dom-to-image-element branch from 7e4518b to 5a49afc Compare August 2, 2023 08:46

Merge branch 'next' into neatbyte/dom-to-image-element

897afbf

fix: fixed api tests

940933b

adrians5j approved these changes Aug 7, 2023

View reviewed changes

adrians5j merged commit 770012a into next Aug 7, 2023
58 checks passed

adrians5j added this to the 5.38.0 milestone Aug 7, 2023

brunozoric pushed a commit that referenced this pull request Oct 10, 2023

fix: render blocks when previewing (instead of using preview image) (#…

116be60

…3329)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: fix element thumbnail visual issues #3329

fix: fix element thumbnail visual issues #3329

neatbyte-vnobis commented May 24, 2023

SvenAlHamad commented May 26, 2023

adrians5j commented May 29, 2023

adrians5j left a comment •

edited

neatbyte-ivelychko commented Jul 11, 2023 •

edited

1. Image issue

neatbyte-ivelychko commented Jul 11, 2023 •

edited

2. Still using image?

adrians5j Jul 11, 2023

adrians5j Jul 18, 2023

neatbyte-ivelychko Jul 18, 2023

neatbyte-ivelychko Aug 2, 2023 •

edited

adrians5j Jul 18, 2023

adrians5j Jul 18, 2023

adrians5j Jul 18, 2023

adrians5j left a comment •

edited

neatbyte-ivelychko commented Jul 18, 2023

adrians5j left a comment

neatbyte-ivelychko commented Jul 26, 2023 •

edited

neatbyte-ivelychko commented Jul 26, 2023

neatbyte-vnobis commented Aug 4, 2023

github-actions bot commented Aug 4, 2023

github-actions bot commented Aug 4, 2023

github-actions bot commented Aug 4, 2023

adrians5j Aug 7, 2023

fix: fix element thumbnail visual issues #3329

fix: fix element thumbnail visual issues #3329

Conversation

neatbyte-vnobis commented May 24, 2023

Changes

How Has This Been Tested?

Documentation

SvenAlHamad commented May 26, 2023

adrians5j commented May 29, 2023

adrians5j left a comment • edited

Choose a reason for hiding this comment

1. Image issue

2. Still using image?

neatbyte-ivelychko commented Jul 11, 2023 • edited

1. Image issue

neatbyte-ivelychko commented Jul 11, 2023 • edited

2. Still using image?

adrians5j Jul 11, 2023

Choose a reason for hiding this comment

adrians5j Jul 18, 2023

Choose a reason for hiding this comment

neatbyte-ivelychko Jul 18, 2023

Choose a reason for hiding this comment

neatbyte-ivelychko Aug 2, 2023 • edited

Choose a reason for hiding this comment

adrians5j Jul 18, 2023

Choose a reason for hiding this comment

adrians5j Jul 18, 2023

Choose a reason for hiding this comment

adrians5j Jul 18, 2023

Choose a reason for hiding this comment

adrians5j left a comment • edited

Choose a reason for hiding this comment

neatbyte-ivelychko commented Jul 18, 2023

adrians5j left a comment

Choose a reason for hiding this comment

neatbyte-ivelychko commented Jul 26, 2023 • edited

neatbyte-ivelychko commented Jul 26, 2023

neatbyte-vnobis commented Aug 4, 2023

github-actions bot commented Aug 4, 2023

github-actions bot commented Aug 4, 2023

github-actions bot commented Aug 4, 2023

adrians5j Aug 7, 2023

Choose a reason for hiding this comment

adrians5j left a comment •

edited

neatbyte-ivelychko commented Jul 11, 2023 •

edited

neatbyte-ivelychko commented Jul 11, 2023 •

edited

neatbyte-ivelychko Aug 2, 2023 •

edited

adrians5j left a comment •

edited

neatbyte-ivelychko commented Jul 26, 2023 •

edited