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

Geometry Preview refactor - separate into several files, allow running in browser directly #628

Merged
merged 6 commits into from
Aug 29, 2023

Conversation

jmarrec
Copy link
Collaborator

@jmarrec jmarrec commented Aug 28, 2023

I find the geometry_preview.html awful to work with, as it has a lot of embedded jS into it.

Also, having the ability to run it directly into your browser is helpful for testing out changes.

This PR:

  • Extract the inline jS from geometry_preview.html into separate jS files
  • Upgrades these dependencies (to a version without major breaking changes), and downloads the unminified version that matches, so one could technically swap to these to debug something
  • Be explicit about which versions of these dependencies we use
  • Allows running the geometry_preview in your browser directly with some json data.
    • Add separate entry points runFromJSON and runFromFile

@macumber
Copy link
Collaborator

macumber commented Aug 29, 2023

This looks great @jmarrec, as an FYI, I've tried to keep the html and code here in sync with:

NREL uses those measures to view models from PAT, etc. If NREL is willing to support you doing it could you consider porting these changes back to the view_model and view_data measures? There could be a potential issue with XSS checks when loading the results of the measures in the results tab?

jmarrec added a commit to NREL/openstudio-common-measures-gem that referenced this pull request Aug 29, 2023
openstudiocoalition/OpenStudioApplication#628
* Extract the inline jS from geometry_preview.html into separate jS files
* Upgrades these dependencies (to a version without major breaking changes), and downloads the unminified version that matches, so one could technically swap to these to debug something
* Be explicit about which versions of these dependencies we use
* Allows running the geometry_preview in your browser directly with some json data.
    * Add separate entry points `runFromJSON` and `runFromFile`

openstudiocoalition/OpenStudioApplication#629
* Opt-in feature to enable the geometry diagnostics from this PR:
@jmarrec
Copy link
Collaborator Author

jmarrec commented Aug 29, 2023

Backported at NREL/openstudio-common-measures-gem#156

Tested the measures locally + inside the OS App (I ended up embedding the jS files + data into the report.html to avoid issues)

@jmarrec jmarrec merged commit 6aee38e into develop Aug 29, 2023
@jmarrec jmarrec deleted the geometry_refactor branch August 29, 2023 09:54
@github-actions github-actions bot locked and limited conversation to collaborators Aug 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants