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.

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

feat: add preview in dashboard using iframe #2032

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

rinczefi-user
Copy link
Contributor

@rinczefi-user rinczefi-user commented Sep 4, 2024

Changes:

  • ♻️ the useResumePreview hook is currently unused, because huge loading times
  • 🚀 I refactored it to use the same functionality as here

Resume/blob/main/apps/client/src/pages/public/page.tsx#L81-L86)

  • using iframe to render it and set resume in the useEffect((), [frameRef])

💪 It is fast and from the exploratory tests I've done locally, we can rely on it, as well.

image

Discussion in issue: #2005 (comment)

@rinczefi-user rinczefi-user mentioned this pull request Sep 4, 2024
1 task
@rinczefi-user
Copy link
Contributor Author

@AmruthPillai please review and merge if it's okay. Thank you!

@rinczefi-user
Copy link
Contributor Author

rinczefi-user commented Sep 5, 2024

Though in case the page's content overflows, the scrollbars will be visible.. 👇
image

The first CV has 2 pages and the first one is too full of content.

This can be resolved with setting the iframe.scrolling to "no".

<iframe
  ref={frameRef}
  title={resume.title}
  src="/artboard/preview"
  scrolling="no"
  style={{
    height: "100%",
    width: "100%",
    zoom: 0.35,
  }}
/>

After the fix:
image

@rinczefi-user
Copy link
Contributor Author

rinczefi-user commented Sep 5, 2024

There is only one deprecation warning, related to iframe.scrolling.
Didn't find a viable alternative, but this one is working as expected.

⚠️ 'scrolling' is deprecated.ts(6385)

commit: 4d4f28d

@rinczefi-user
Copy link
Contributor Author

Review please 🙏
Thank you! 😄

cc. @AmruthPillai

@rinczefi-user
Copy link
Contributor Author

Review please 🙏 Thank you! 😄

cc. @AmruthPillai

@AmruthPillai What do you think?

@rinczefi-user
Copy link
Contributor Author

Review please 🙏 Thank you! 😄
cc. @AmruthPillai

@AmruthPillai What do you think?

Small reminder 😄 Thank you!

cc. @AmruthPillai

@rinczefi-user
Copy link
Contributor Author

@AmruthPillai Please review 😄

@ektorasdj
Copy link

Hello @rinczefi-user !
I have tested this in my setup but the preview is zoomed in.
image
I have done all your changes as shown in your commits.

@ektorasdj
Copy link

It seems that this happens on Firefox. On Google chrome it shows ok.

@rinczefi-user
Copy link
Contributor Author

It seems that this happens on Firefox. On Google chrome it shows ok.

Hmm.. I see that it's supported only in the new Firefox versions.. 🤔

image

@rinczefi-user
Copy link
Contributor Author

I'll try this suggestion 👇 later today, thanks!
https://stackoverflow.com/questions/4049342/how-can-i-zoom-an-html-element-in-firefox-and-opera

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants