Sidebar optimisation as separate panel instead of overlay #289

peteward · 2023-10-25T16:04:45Z

implements #248

IMPROVEMENTS:

Use a dedicated panel for the sidebar, instead of an overlay, to give more effective use of space.

Fix a bug where the container would not appropriately resize (with the correct canvas size for scrollbars) after zooming until an element was selected.

Screen.Recording.2023-10-26.at.13.22.34.mov

Fix a bug where the left-hand side of content will be cropped:

Screen.Recording.2023-10-26.at.13.20.58.mov

Fix double (and sometimes triple) scrollbar issues:

peteward · 2023-10-25T16:05:30Z

packages/ui/src/components/CtlBar/index.tsx

@@ -35,10 +35,7 @@ const CtlBar = (props: Props) => {
        }}
      >
        {pageNum > 1 && (
-          <>
-            <Pager pageCursor={pageCursor} pageNum={pageNum} setPageCursor={setPageCursor} />
-            <strong style={{ color: 'white', fontSize: '0.9rem', padding: 0 }}>|</strong>


I removed the | separator as I thought it looked better with just space but I can re-add

peteward · 2023-10-25T16:07:13Z

packages/ui/src/components/Designer/index.tsx

@@ -247,12 +253,7 @@ Check this document: https://pdfme.com/docs/custom-schemas`);
          onEditEnd();
        }}
        zoomLevel={zoomLevel}
-        setZoomLevel={(zoom) => {
-          if (mainRef.current) {
-            mainRef.current.scrollTop = getPagesScrollTopByIndex(pageSizes, pageCursor, scale);


I tried to align the left scroll centrally but this did not seem worth the effort.
Similarly, I didn't think pulling the viewport to the top was good UX, it felt OK to just zoom in / out from where you were.

peteward · 2023-10-25T16:07:37Z

packages/ui/src/components/Paper.tsx

-        ...size,
+        transformOrigin: 'top left',
+        // NOTE: These values do not apply, but changing them with scale ensures the
+        // container is updated to show appropriately sized scrollbars when you zoom in/out.


there is probably a better way of doing this

packages/ui/src/components/Designer/Main/index.tsx

hand-dot · 2023-10-25T23:58:39Z

test and reviewing

hand-dot · 2023-10-26T00:08:28Z

Also, I'll fix main branch failing test. sorry

hand-dot · 2023-10-26T00:32:37Z

Very cool. this is a ideal sidebar position...! thank you.

but i fond bug, when i set multi page pdf template to form or viewer, PDF pages overflow the root element.
I gonna investigate code changes.

also, when i set multi page pdf as besePdf to designer, we can see wrong position CtlBar.

you can reproduce from this template.json.

This behavior should be maintained.

1026.1.mov

So, we need check this pattern

hand-dot · 2023-10-26T00:39:49Z

packages/ui/src/components/CtlBar/index.tsx

Please explain the changes to this file.

The CtlBar is absolutely positioned to its container.
The outer div places the CtrlBar in the correct place.
The inner div no longer needs any special positioning. Sticky is not required because the div is absolute to the parent and does not move.

…r-optimisation

hand-dot · 2023-10-26T03:46:20Z

↑ Pulled the main branch and incorporated fixes for tests and rotation.
@peteward can you pull latest 248-sidebar-optimisation?

peteward · 2023-10-26T09:52:55Z

I'm looking into this now 👀

consolidate use of sizeExclSizebar

peteward · 2023-10-26T13:45:16Z

I think this is all working fine now - I have updated the PR description with information about the changes.

I didn't have any problem with the position of the CtrlBar though on the Designer, I'm not sure how you were seeing that 🤔

Single Page Designer:

Screen.Recording.2023-10-26.at.14.39.22.mov

peteward · 2023-10-26T13:45:42Z

Single Page Viewer:

Screen.Recording.2023-10-26.at.14.40.45.mov

peteward · 2023-10-26T13:46:44Z

Multi-page Designer:

Screen.Recording.2023-10-26.at.14.46.01.mov

peteward · 2023-10-26T13:47:44Z

Multi-page Viewer:

Screen.Recording.2023-10-26.at.14.47.08.mov

peteward · 2023-10-26T13:51:04Z

packages/ui/src/components/Designer/Canvas/index.tsx

 }

-const Main = (props: Props, ref: Ref<HTMLDivElement>) => {
+const Canvas = (props: Props, ref: Ref<HTMLDivElement>) => {


I renamed this to Canvas as it felt more specific than Main, but I will revert if you disagree.

peteward · 2023-10-26T13:51:51Z

packages/ui/src/components/Designer/Canvas/index.tsx

@@ -247,7 +253,15 @@ const Main = (props: Props, ref: Ref<HTMLDivElement>) => {
    .every((schema) => schema.rotate !== undefined);

  return (
-    <div ref={ref} style={{ overflow: 'overlay' }}>


overflow: overlay was deprecated a while ago and does not have consistent support. We're using overflow: scroll or auto now where needed.

peteward · 2023-10-26T13:57:40Z

packages/ui/src/components/Preview.tsx

-          );
-        }}
-      />
+      <div ref={containerRef} style={{ ...size, position: 'relative', overflow: 'auto' }}>


I have changed Preview to be consistent with the Designer, with a wrapping Div which is the viewport/canvas for scrolling.

peteward · 2023-10-26T14:18:35Z

Hi @hand-dot,

Are these tests failing because it's taking a long time to run the generator integration tests?

I don't think I have made any changes here that would impact this... 🤔

hand-dot · 2023-10-26T14:24:05Z

Hi @hand-dot,

Are these tests failing because it's taking a long time to run the generator integration tests?

I don't think I have made any changes here that would impact this... 🤔

Sometime it occurs. Let's run again

hand-dot · 2023-10-26T14:32:30Z

Okay test is passing, I gonna check code tomorrow

hand-dot

The UX of pdfme has significantly improved.

It's Saturday night in Japan right now, and I can enjoy a beer in the best mood.
Thank you for the amazing PR.

Sidebar optimisation as separate panel instead of overlay

6f2047c

peteward commented Oct 25, 2023

View reviewed changes

packages/ui/src/components/Designer/Main/index.tsx Outdated Show resolved Hide resolved

hand-dot reviewed Oct 26, 2023

View reviewed changes

Merge branch 'main' of https://github.com/pdfme/pdfme into 248-sideba…

5a2c845

…r-optimisation

f

f0b4ecb

peteward added 2 commits October 26, 2023 12:45

f

abf842a

rename Main -> Canvas

a9d7473

consolidate use of sizeExclSizebar

peteward commented Oct 26, 2023

View reviewed changes

test snapshot fixes

ca70f90

peteward mentioned this pull request Oct 26, 2023

Zooming in on viewer cuts off left side of document #189

Closed

fix canvas size for low res PDFs where scale starts at 0

b024e35

peteward mentioned this pull request Oct 26, 2023

Pager is off, causing multi page pdfs to be cut off at the top #155

Closed

hand-dot approved these changes Oct 27, 2023

View reviewed changes

hand-dot merged commit 36041d8 into main Oct 27, 2023
2 checks passed

hand-dot deleted the 248-sidebar-optimisation branch November 9, 2023 00:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sidebar optimisation as separate panel instead of overlay #289

Sidebar optimisation as separate panel instead of overlay #289

peteward commented Oct 25, 2023 •

edited

Loading

peteward Oct 25, 2023

peteward Oct 25, 2023

peteward Oct 25, 2023

hand-dot commented Oct 25, 2023

hand-dot commented Oct 26, 2023

hand-dot commented Oct 26, 2023 •

edited by peteward

Loading

hand-dot Oct 26, 2023

peteward Oct 26, 2023

hand-dot commented Oct 26, 2023 •

edited

Loading

peteward commented Oct 26, 2023

peteward commented Oct 26, 2023

peteward commented Oct 26, 2023

peteward commented Oct 26, 2023

peteward commented Oct 26, 2023

peteward Oct 26, 2023

peteward Oct 26, 2023 •

edited

Loading

peteward Oct 26, 2023 •

edited

Loading

peteward commented Oct 26, 2023

hand-dot commented Oct 26, 2023

hand-dot commented Oct 26, 2023

hand-dot left a comment

Sidebar optimisation as separate panel instead of overlay #289

Sidebar optimisation as separate panel instead of overlay #289

Conversation

peteward commented Oct 25, 2023 • edited Loading

IMPROVEMENTS:

peteward Oct 25, 2023

Choose a reason for hiding this comment

peteward Oct 25, 2023

Choose a reason for hiding this comment

peteward Oct 25, 2023

Choose a reason for hiding this comment

hand-dot commented Oct 25, 2023

hand-dot commented Oct 26, 2023

hand-dot commented Oct 26, 2023 • edited by peteward Loading

hand-dot Oct 26, 2023

Choose a reason for hiding this comment

peteward Oct 26, 2023

Choose a reason for hiding this comment

hand-dot commented Oct 26, 2023 • edited Loading

peteward commented Oct 26, 2023

peteward commented Oct 26, 2023

peteward commented Oct 26, 2023

peteward commented Oct 26, 2023

peteward commented Oct 26, 2023

peteward Oct 26, 2023

Choose a reason for hiding this comment

peteward Oct 26, 2023 • edited Loading

Choose a reason for hiding this comment

peteward Oct 26, 2023 • edited Loading

Choose a reason for hiding this comment

peteward commented Oct 26, 2023

hand-dot commented Oct 26, 2023

hand-dot commented Oct 26, 2023

hand-dot left a comment

Choose a reason for hiding this comment

peteward commented Oct 25, 2023 •

edited

Loading

hand-dot commented Oct 26, 2023 •

edited by peteward

Loading

hand-dot commented Oct 26, 2023 •

edited

Loading

peteward Oct 26, 2023 •

edited

Loading

peteward Oct 26, 2023 •

edited

Loading