Refactor ModalBase from existing modals

[anthonyshibitov]

Refactor ModalBase from existing modals

ModalBase can accept an imageModal prop to change behavior and match existing functionality.

Contributor checklist

• This pull request is on a separate branch and not the main branch

---

Description

Refactor modal functionality to a separate ModalBase.vue component.

imageBase can be added to the <ModalBase> component to mimic the functionality we currently have, where images and text modals behave (click image to close modal) and display (text modal has close button and title in a card, with the content within it) differently.

Note that this currently does not support i8n in the modal title (specifically, {{ $t("components.modal-qr-code.header") }}). I wanted to get some feedback on this before fleshing out the remaining portions, to make sure this design is what we're looking for. Let me know if this is okay, and I will commit another change to try and fix the i8n issue (I'm currently not sure how to do this, any ideas would be great lol 😀). If we want the modal to behave differently, I'll go back to the drawing board.

Also, note to myself, the bug where clicking to the left or right of the image modal does not close the modal still exists 🙃

Related issue

• Create base modal component #582

[netlify]

✅ Deploy Preview for activist-org ready!

Name	Link
🔨 Latest commit	47b3454
🔍 Latest deploy log	https://app.netlify.com/sites/activist-org/deploys/656e58f04185b70008902f60
😎 Deploy Preview	https://deploy-preview-595--activist-org.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[github-actions]

Thank you for the pull request!

The activist team will do our best to address your contribution as soon as we can. The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :)

If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and Development rooms once you're in. It'd be great to have you!

Maintainer checklist

• The commit messages for the remote branch should be checked to make sure the contributor's email is set up correctly so that they receive credit for their contribution

  • The contributor's name and icon in remote commits should be the same as what appears in the PR
  • If there's a mismatch, the contributor needs to make sure that the email they use for GitHub matches what they have for git config user.email in their local activist repo

• The TypeScript and formatting workflows within the PR checks do not indicate new errors in the files changed

• The CHANGELOG has been updated with a description of the changes for the upcoming release and the corresponding issue (if necessary)

[andrewtavis]

Thanks @anthonyshibitov! I'll check this in the coming days and provide some feedback 😊

[andrewtavis]

Looking good so far, @anthonyshibitov! I'm going to wait on #565 to get merged before bringing this in. Some thoughts on this:

• For the image maybe removing the active state when it's expanded would be good as when the user clicks the image to close it gets a border around it, but clicking the sides it won't (maybe will be solved by the change to get the left and right parts of the expanded modal to work to close the image)
• Looks like you're using npm to set things up rather than yarn, which is what it is, but I'd warn against installing any dependencies in this way as we've found that npm installed dependencies can break things
  • Regardless, can you revert the package-lock.json files 🙃
• Re you're question on the i18n changes to this:
  • I think this could make sense, or maybe we can also just drop all the card contents including the title into s <slot>? Might be a bit weird with the close icon, but that way all the logic for the modal card would be in the slot and we wouldn't also have to assign some parts in the props.
  • Lemme know what you think! It'll work either way :)

[anthonyshibitov]

Looking good so far, @anthonyshibitov! I'm going to wait on #565 to get merged before bringing this in. Some thoughts on this:

• For the image maybe removing the active state when it's expanded would be good as when the user clicks the image to close it gets a border around it, but clicking the sides it won't (maybe will be solved by the change to get the left and right parts of the expanded modal to work to close the image)

• Looks like you're using npm to set things up rather than yarn, which is what it is, but I'd warn against installing any dependencies in this way as we've found that npm installed dependencies can break things

  • Regardless, can you revert the package-lock.json files 🙃

• Re you're question on the i18n changes to this:

  • I think this could make sense, or maybe we can also just drop all the card contents including the title into s <slot>? Might be a bit weird with the close icon, but that way all the logic for the modal card would be in the slot and we wouldn't also have to assign some parts in the props.
  • Lemme know what you think! It'll work either way :)

Alrighty, looks like I've got points 2 and 3 all set 😎 I'm not sure what you mean by removing the active state from the image though, could you explain that?

[andrewtavis]

I can take a look at it myself, but when I click the image to close it there's a border that appears around it, whereas when we'd click the sides there wouldn't be. Trying to make sure that the closing modal experience is the same :)

[andrewtavis]

Are we good for a final review on this, @anthonyshibitov? :)

[anthonyshibitov]

Are we good for a final review on this, @anthonyshibitov? :)

yessir!

[andrewtavis]

Wunderbar 😊 I'll get to it soon then :)

[andrewtavis]

This is some great stuff here, @anthonyshibitov :)

[andrewtavis]

Really well organized. The double multiple slots that are named makes this super easy to follow 😊

[andrewtavis]

Thanks for this, @anthonyshibitov! I'll try to play around to get the image boundaries to be clickable to close it :)