activist
activist copied to clipboard
923 - Modal still open ...
Contributor checklist
- [x] This pull request is on a separate branch and not the main branch
Description
This PR addresses issue 923. The changes affected modal components that use an 'isOpen' prop; the 'isOpen' props and related functionality were removed.
Modals related to Organizations were affected. Other entities (ie Events) were not checked.
Testing was done following the original description of the problem in issue 923 (click activist icon, use forward / backward arrows to navigate, etc.). However, there appears to be an error with reloading the organization data when clicking the back button to check if modal is not displayed, so we can't be completely sure that everything is fixed.
That being said, the affected modals appear to work correctly without the 'isOpen' props, meaning that they appear to operate normally.
Related issue
- #923
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. Also consider joining our bi-weekly Saturday dev syncs. It'd be great to have you!
Maintainer checklist
-
[x] 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.emailin their local activist repo
-
[x] The TypeScript and formatting workflows within the PR checks do not indicate new errors in the files changed
-
[x] The Playwright end to end and Zap penetration tests have been ran and are passing (if necessary)
-
[x] The CHANGELOG has been updated with a description of the changes for the upcoming release and the corresponding issue (if necessary)
Deploy Preview for activist-org ready!
| Name | Link |
|---|---|
| Latest commit | 9cbe8d84eb944afef7a4e4fa24b7a3466358d4d6 |
| Latest deploy log | https://app.netlify.com/sites/activist-org/deploys/66b8f5059220c600083bcb24 |
| Deploy Preview | https://deploy-preview-941--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.
One question on this, @mattburnett-repo: What the purpose of all the Watch for closeModal emit and do cleanup when it happens. in the codebase now? Can we remove these somehow? And can we clean up the command palette code a bit to remove the TODOs from it? Maybe you could let me know what issues could be made from those TODOs, and then I can make the issues and we can remove the comments?
The 'Watch for closeModal emit...' are there just in case they are needed in the future. If you prefer, they can be removed.
The TODO messages in the command palette are notes-to-self. Later this evening I will go through them and let you know which of these are issue-relevant.
Here's a list of issues from the TODO's removed from ModalCommandPalette.vue
Search works, but it's picking up the literal values of 'displayName' (eg. "_global.home"). Search should instead look through the rendered values.
Sometimes router.push() loses the i18n part of the path (eg: /en/the.path.name)