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

Require confirmation to delete WebVTT #4232

Merged
merged 1 commit into from
Oct 23, 2024
Merged

Require confirmation to delete WebVTT #4232

merged 1 commit into from
Oct 23, 2024

Conversation

kdid
Copy link
Contributor

@kdid kdid commented Oct 23, 2024

Summary

Presents users with a confirmation message and button when they delete WebVTT content.

Screenshot 2024-10-23 at 6 50 43 AM

Specific Changes in this PR

  • Require confirmation before deleting WebVTT content in the UI associated with Audio and Video files

Version bump required by the PR

See Semantic Versioning 2.0.0 for help discerning which is required.

  • Patch
  • Minor
  • Major

Steps to Test

  • Go to a work page in Meadow where a file set has existing WebVTT content
  • Click the "Edit WebVTT structure" button
  • Click "Delete WebVTT"
  • Confirm you are presented with a warning message asking if you are sure you want to delete.
  • Click "Yes, delete"
  • Confirm the content is deleted
  • Repeat above steps, except instead of confirming, click "Cancel"
  • Confirm the content is not deleted

🚀 Deployment Notes

Note - if you check any of these boxes go to the (always open) main <- staging PR and add detailed notes and instructions to help out others who may end up deploying your changes to production

  • Backward compatible API changes
    • Database Schema changes
    • GraphQL API
    • Elasticsearch API
    • Ingest Sheet
    • CSV metadata export/update API
    • Shared Links export API
  • Backwards-incompatible API changes
    • Database Schema changes
    • GraphQL API
    • Elasticsearch API
    • Ingest Sheet
    • CSV metadata export/update API
    • Shared Links export API
  • Requires data migration
  • Requires database triggers disabled during deployment/migration
  • Requires reindex
  • Terraform changes
    • Adds/requires new or changed Terraform variables
  • Pipeline configuration changes (requires mix meadow.pipeline.setup run)
  • Requires new variable added to miscellany
  • Specific deployment synchronization instructions with other apps/API's
  • Other specific instructions/tasks

Tested/Verified

  • End users/stakeholders

@kdid kdid self-assigned this Oct 23, 2024
Copy link
Member

@mathewjordan mathewjordan left a comment

Choose a reason for hiding this comment

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

I really like this!

Not a big deal, but one thing I think we could add. Take or leave it? On line ~124, we could add this to hide away the textarea during the confirmation.

<textarea
  className="textarea"
  onChange={handleChange}
  placeholder="Enter WebVTT text here"
  ref={textAreaRef}
  rows="10"
- style={{ whiteSpace: "pre-wrap" }}
+ style={{
+  whiteSpace: "pre-wrap",
+  display: confirmDelete ? "none" : "block",
+ }}
  value={webVttValue}
/>

@kdid
Copy link
Contributor Author

kdid commented Oct 23, 2024

@mathewjordan - this is updated. Now looks like this 👇 . Thanks!

Screenshot 2024-10-23 at 7 57 56 AM

@kdid kdid merged commit 227354f into deploy/staging Oct 23, 2024
4 checks passed
@kdid kdid deleted the 4119-delete-vtt branch October 23, 2024 15:20
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