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

Integrate config editor into RESPECT #271

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

mohsin-r
Copy link
Member

@mohsin-r mohsin-r commented Mar 21, 2024

Related Item(s)

Closes #246.
Closes #256.

Changes

  • Replaced FGP Authoring Tool with the config editor.
  • Created NPM package for the config editor.
  • Refactored styling across the app.

Notes

This PR results in breaking changes, namely the map panel for any existing products with RAMP2 configs in them will no longer work. The rest of the editor should work, however.
Additionally, maps don't work when you preview the whole product. This has been logged in the storylines repo. For now, maps can be previewed using the preview functionality in the config editor.
Also, the options section in the config editor cannot be saved. This is because our current storylines schema doesn't support it, so I didn't save it anywhere. Just in case someone grouses that's a bug.

Testing

Steps:

  1. Open the storylines editor.
  2. Attempt to create, load, and save maps with different configurations.
  3. Grouse if something doesn't work. Since styling was refactored, a quick look at overall styling would also be appreciated.

This change is Reviewable

Copy link

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

Copy link
Member

@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.

Nice work 🏆

Looks like the preview mode styling needs to be fixed. Might need to modify the tailwind configuration is my initial guess.

Reviewed 1 of 230 files at r1.
Reviewable status: 1 of 230 files reviewed, 4 unresolved discussions (waiting on @mohsin-r)


public/help/en.md line 91 at r1 (raw file):

| ![The icon representing the Toggle Visibility function](layer/toggleVisibility.png) | Toggle Visibility | Enables or disables the visibility for all layers |

There is an updated version of the RAMP help docs


src/components/editor/metadata-editor.vue line 10 at r1 (raw file):

                        {{ editExisting ? $t('editor.editProduct') : $t('editor.createProduct') }}
                    </div>
                    <button class="editor-button" @click="swapLang()">

This button should be right-aligned on the metadata page


src/components/editor/metadata-editor.vue line 83 at r1 (raw file):

            <div class="flex mt-8">
                <button v-if="editExisting" @click="saveMetadata(true)" class="editor-button pl-8">

Remove "pl-8"


src/components/editor/metadata-editor.vue line 92 at r1 (raw file):

                    <button
                        @click="!warning ? continueToEditor() : $vfm.open(`confirm-uuid-overwrite`)"
                        class="editor-button bg-black text-white px-8"

Remove "px-8"

Copy link
Member Author

@mohsin-r mohsin-r left a comment

Choose a reason for hiding this comment

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

Donethanks by updating to the latest storylines package.

Reviewable status: 0 of 232 files reviewed, 4 unresolved discussions (waiting on @yileifeng)


public/help/en.md line 91 at r1 (raw file):

Previously, yileifeng (Yi Lei Feng) wrote…

There is an updated version of the RAMP help docs

Donethanks.


src/components/editor/metadata-editor.vue line 10 at r1 (raw file):

Previously, yileifeng (Yi Lei Feng) wrote…

This button should be right-aligned on the metadata page

Donethanks.


src/components/editor/metadata-editor.vue line 83 at r1 (raw file):

Previously, yileifeng (Yi Lei Feng) wrote…

Remove "pl-8"

Donethanks.


src/components/editor/metadata-editor.vue line 92 at r1 (raw file):

Previously, yileifeng (Yi Lei Feng) wrote…

Remove "px-8"

Donethanks.

Copy link
Member

@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.

Reviewed 222 of 230 files at r1, 10 of 10 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @mohsin-r)

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.

Looks really good, awesome work!

A couple of things:

  1. The preview seems to break when you modify the starting fixtures. I tried to remove the export fixture and when I pressed preview, the map loaded but the fixtures did not. It also seems like extra spaces are added in between the list items when you make changes to it. When I save the product and look at the config the array looks like this, so stripping probably has to be done somewhere:
"startingFixtures": [
  "geosearch",
  " overviewmap",
  " basemap",
  ...
],
  1. When using RESPECT in French, the map editor remains in English.

Reviewed 222 of 230 files at r1, 10 of 10 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @mohsin-r)

Copy link
Member Author

@mohsin-r mohsin-r left a comment

Choose a reason for hiding this comment

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

  1. This should be fixed now. Thanks for the catch!
  2. Dan mentioned that French translations for the config editor are not urgent and it can remain in English for now. However, I do agree that we should at least have the [FR] placeholder appended to English text. Since this will take a while to do, I think it would be better to make a new issue for this, as opposed to adding it in this PR.

Reviewable status: 220 of 232 files reviewed, all discussions resolved (waiting on @RyanCoulsonCA and @yileifeng)

Copy link
Member

@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.

Reviewed 12 of 12 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @mohsin-r)

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.

Sounds good to me!

Reviewed 12 of 12 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @mohsin-r)

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 216 of 230 files at r1, 4 of 10 files at r2, 12 of 12 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @mohsin-r)

Copy link
Member

@KashishMistry KashishMistry left a comment

Choose a reason for hiding this comment

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

Reviewed 216 of 230 files at r1, 4 of 10 files at r2, 12 of 12 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @mohsin-r)

@RyanCoulsonCA RyanCoulsonCA merged commit 63e78cc into ramp4-pcar4:main Apr 16, 2024
3 checks passed
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.

Saving and integrating the config-editor Package the config-editor to auto-build with RESPECT
5 participants