Closed Robot8lover closed 2 weeks ago
Name | Link |
---|---|
Latest commit | 783291cc0265c85eb0e9d4619e14881d796e9068 |
Latest deploy log | https://app.netlify.com/sites/activist-org/deploys/666ef1cdb9aa9d0008cdba79 |
Deploy Preview | https://deploy-preview-887--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.
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!
[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
git config user.email
in 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)
Thanks for this, @Robot8lover! Will get to it by early next week at the latest :)
Something that I'm realizing here @Robot8lover is that no matter what ID we're on we're returning 1
for the ID for the QR code... This is the case for organizations, groups and events. Would you have interest in looking into this?
The only other comment is on the way the the Open image in a new tab
label flickers when we hover over it itself. Could we figure out a fix for this? Specifically the cursor tends to be over it when the QR code modal is opened, so then the first thing the user experiences is the label flickering.
Really appreciate all the work here!
Also, let me know if you can see the errors in the workflows. Would be happy to assist with those as well! I'll be away for the weekend, but will be happy to assist and bring this in next week!
@andrewtavis I will take a look at that. And yes, I can see the errors, and I'll try to fix those.
Thanks @Robot8lover :) Let me know if there's anything I can do to help!
@andrewtavis I'm a bit confused what you mean when you say that no matter what ID we're on we're returning 1 for the ID. Where does this ID show up?
Oh, I think I know what you mean βΒ the QR codes point to /en/events/1 (or organizations/1 or whatever it is) instead of what they actually are from. This is based on linkURL, which is based on useLinkURL from frontend/composables/useLinkURL.ts, which bases itself off of the props object. Within frontend/pages/organizations/[id]/about.vue and the other organizations/[id] pages is the line const organization = testTechOrg
so I believe that that's what is setting the id to 1. This would no doubt change in production though I'm not quite sure how it works yet and what creating an organization/event would actually do. The QR code does indeed point to a different location if you change the value of getPath() on the QRCodeImage, so that part works fine.
Fantastic and totally makes sense, @Robot8lover! Was worried we had something hard coded, but if it's based on the ID of the object then we're good :) Thanks for the investigation!
Will get to the full review in the coming days π
Contributor checklist
Description
Introduce basic functionality for opening a the QR code in a new tab as request in #645. Opens a new window containing an HTMLImageElement with the QR code as a png data url src. Tested on Chrome and Safari, and it works as expected though pop-up windows must be allowed.
Related issue
645