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

dynamic panel editor enhancements #358

Merged
merged 1 commit into from
Jul 29, 2024
Merged

Conversation

RyanCoulsonCA
Copy link
Member

@RyanCoulsonCA RyanCoulsonCA commented Jul 8, 2024

Related Item(s)

#342

Changes

  • Disable the "Add New" button if the ID isn't filled out when adding a new panel.
  • Fix issue with duplicate ID string being cut off.
  • Added an additional confirmation step before deleting subpanels.

Testing

Steps:

  1. Open the demo page and load a product.
  2. Create a dynamic panel and add some subpanels.
  3. Try to add a new subpanel with no name and ensure that you can't click Add new.
  4. Try to add a new subpanel with the same name as another subpanel. Ensure the error message pops up and it's not cut off.
  5. Try to remove a subpanel and ensure the additional confirmation message pops before it is removed.

This change is Reviewable

Copy link

github-actions bot commented Jul 8, 2024

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

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.

Found two issues when testing:

  1. After creating a new dynamic panel, the first subpanel cannot be deleted (no action when removing).
  2. When accessing an existing dynamic panel on the 0000 product (second slide), the config seems to be breaking the schema validator (resulting in other issues such as not being able to delete the slide). This also seems to happen when creating a new empty dynamic panel:
    image.png

image copy 1.png

Reviewable status: 0 of 2 files reviewed, all discussions resolved

Copy link
Member Author

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

I've fixed the issue with deleting the first subpanel, and the issue with the new dynamic panel showing an error in the advanced editor. That version of the 0000 product seems to be quite outdated (notice that the slideshow entry still has the images property instead of items), which is why that's erroring.

Reviewable status: 0 of 3 files reviewed, all discussions resolved

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 3 of 3 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @RyanCoulsonCA)

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 3 of 3 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @RyanCoulsonCA)

@yileifeng yileifeng merged commit 1bab84d into ramp4-pcar4:main Jul 29, 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.

3 participants