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

replace RAMP build files with npm #363

Merged
merged 2 commits into from
Aug 21, 2024
Merged

Conversation

yileifeng
Copy link
Member

@yileifeng yileifeng commented Jul 15, 2024

Related Item(s)

Closes #326, #354

Changes

  • Removed RAMP build files and integrated RAMP through NPM package

Notes

Testing

RAMP config editor outputs maps that can be previewed correctly.


This change is Reviewable

Copy link

Your demo site is ready! 🚀 Visit it here: https://ramp4-pcar4.github.io/storylines-editor/ramp-npm

Copy link
Member

@RyanCoulsonCA RyanCoulsonCA left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 9 files reviewed, 1 unresolved discussion (waiting on @yileifeng)


src/components/map-editor.vue line 54 at r1 (raw file):

import defaultConfig from '../../ramp-default.json';
import TimeSliderEditorV from './helpers/time-slider-editor.vue';
import { createInstance } from 'ramp-config-editor_editeur-config-pcar';

I believe this function is different than the one in the ramp-pcar package and it instead instantiates the editor. Currently if you go to the map editor panel, it's just displaying a map instead of the map editor.

Copy link
Member Author

@yileifeng yileifeng left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 9 files reviewed, 1 unresolved discussion (waiting on @RyanCoulsonCA)


src/components/map-editor.vue line 54 at r1 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

I believe this function is different than the one in the ramp-pcar package and it instead instantiates the editor. Currently if you go to the map editor panel, it's just displaying a map instead of the map editor.

Good catch reverted this

Copy link
Member

@RyanCoulsonCA RyanCoulsonCA left a comment

Choose a reason for hiding this comment

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

Reviewed 8 of 9 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @yileifeng)

Copy link
Member

@szczz szczz left a comment

Choose a reason for hiding this comment

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

Reviewed 8 of 9 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @yileifeng)

Copy link
Member

@dnlnashed dnlnashed left a comment

Choose a reason for hiding this comment

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

Reviewed 8 of 9 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @yileifeng)

Copy link
Member Author

@yileifeng yileifeng left a comment

Choose a reason for hiding this comment

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

Also included the updated NPM Storylines plugin changes in this PR (see ramp4-pcar4/story-ramp#455). Preview mode should be functional for all panel types, and maps should load without issues. Any additional testing on this would be helpful.

Reviewable status: 6 of 10 files reviewed, all discussions resolved (waiting on @dnlnashed, @RyanCoulsonCA, and @szczz)

@yileifeng yileifeng merged commit 03142a9 into ramp4-pcar4:main Aug 21, 2024
2 of 3 checks passed
@yileifeng yileifeng deleted the ramp-npm branch August 21, 2024 12:55
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.

RAMP v4.5.0
4 participants