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

Embed PEV2 plan viewer into Explain -> Visualize #9

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

Conversation

malclocke
Copy link

@malclocke malclocke commented Jul 18, 2024

Embeds the PEV2 plan viewer into the Explain -> Visualize page.

image

The previous instructions with a link to PEV version 1 are still below the new viewer in a collapsed <summary> block.

@malclocke malclocke marked this pull request as draft July 18, 2024 04:05
/>
<link rel="stylesheet" href="https://unpkg.com/pev2/dist/style.css" />

<div id="pev2" class="d-flex flex-column" style="height: 50vh">

Choose a reason for hiding this comment

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

Styles look good to me! Just wondering what d-flex is though? I can't tell if it's a typo of a special type of flexbox 😅

Copy link
Author

Choose a reason for hiding this comment

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

Just wondering what d-flex is though?

Maybe a bootstrap thing? https://getbootstrap.com/docs/4.0/utilities/flex/

Choose a reason for hiding this comment

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

Ohh true, I forgot the admin uses bootstrap

Copy link
Author

Choose a reason for hiding this comment

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

Yeh, but also this thing includes it's own bootstrap on line 7 😅

I literally pulled this code verbatim from https://github.com/dalibo/pev2?tab=readme-ov-file#without-building-tools as a hacky way to see if it worked.

It feels very ugly, I think pghero is already in an iframe in the admin, and then this partial is also loading bootstrap and vuejs 🤮

I don't have the FE skills to do this cleaner, so open to suggestions. Maybe it doesn't matter if it's ugly 🤷

Choose a reason for hiding this comment

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

Actually, I don't think our admin uses bootstrap haha. So at least it's not double loaded.
I feel like this is fine, if there is no other simple way to imbed pev2 without vue then this is probably the best way to do it without spending ages on it.

@malclocke malclocke changed the title WIP Embed PEV2 plan viewer into Explain -> Visualize Aug 7, 2024
@malclocke malclocke marked this pull request as ready for review August 7, 2024 22:27
@malclocke malclocke requested a review from pda August 7, 2024 22:30
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