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

QR-Code download as SVG, JPEG or PNG with selection over dropdownmenu #565

Merged
merged 6 commits into from
Jan 6, 2024

Conversation

mjiruobe
Copy link
Contributor

Hello,
I tried to solve the Issue 474: #473

This was the task:
I should implement a functionality, which allows a user to download the qr code of an event in different formats like svg, png or jpeg. PNG was already implemented by using html2canvas, but the disadvantage of this method was taking a screenshot and you can't use this as an svg.

So the solution was to build the qr code not with half html/css and half a graphic like before, instead i visualized the qr code completely as an svg including the logo, the white circle behind it, the border, the activist.org text with red hat display font and the background of it. The plain qr code already shipped as svg by the library qrcode-vue we used.

The font for the activist.org text was encoded as base64 because embedding google fonts or a web page as font provider is in my opinon the wrong decision for an activist page, which has high standards for data protection.

Problems which i did ran into:

  • transform:translate seems to work different on different browsers? and this was necessary to use less calculations
  • it seems like svg properties for text baseline are different in microsoft edge and ie than in other browsers
  • fonts have to be included into an svg
  • i used position:fixed for the dropdownbutton menuitemlist to make it overlap the modal on desktop; on mobile it had side effects but as there is no "modal" on mobile phones, so position:fixed can be excluded on phones (thanks coding night)
    and some other design bugs

Please give me some code review, i will correct the changes (i think it will be necessary) and then i can create a documentation especially for the QrCode.vue because i did a lot of calculations.

I never worked with Vue.js before or on an open source project and I'm not really a web developer, so please the patience but advices are welcome

Thank you for the possibility to work on such a great project.

Greetings
Mike

Copy link

netlify bot commented Nov 18, 2023

Deploy Preview for activist-org ready!

Name Link
🔨 Latest commit 6e637a9
🔍 Latest deploy log https://app.netlify.com/sites/activist-org/deploys/65991539fcd687000831f5c9
😎 Deploy Preview https://deploy-preview-565--activist-org.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

Copy link
Contributor

github-actions bot commented Nov 18, 2023

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 andrewtavis self-requested a review November 18, 2023 23:14
@andrewtavis
Copy link
Member

Thanks @mjiruobe! Will give it a look as soon as I can 😊

@andrewtavis
Copy link
Member

andrewtavis commented Nov 22, 2023

Hey @mjiruobe 👋 Generally this is really really good :) :) Especially the SVG is amazing! There are some minor things that I'll get to after merging it in, but I did want to check on if you have an idea on how to improve the quality of the activist a and activist.org in the PNG and JPEG exports. Is the conversion of SVG just not leading to high resolution? Could we increase the size of the base SVG such that PNGs and JPEGs that come from it are higher resolution?

Let me know if you have any thoughts on this! Happy to merge after and I can do some adjustments to the dropdown styling, etc 😊

@andrewtavis
Copy link
Member

andrewtavis commented Nov 22, 2023

I'll fix merge conflicts btw, @mjiruobe 😊

@mjiruobe
Copy link
Contributor Author

mjiruobe commented Dec 2, 2023

I fixed the quality loss bug with a specified higher resolution of the png and jpeg's. I also added the calculcated resolution to the dropdownmenu.

If there are any fixes, which i should do before you can integrate the changes, just tell me.

@mjiruobe
Copy link
Contributor Author

mjiruobe commented Dec 2, 2023

I see the BtnLabeled.vue needs a fix. I will go for this little issue soon.

@andrewtavis
Copy link
Member

Should be good from here, @mjiruobe! Thanks for the changes 😊😊

@andrewtavis
Copy link
Member

Ah the right rounding being removed? Sure if you can figure it out by all means :) Also happy to take a look myself.

@andrewtavis
Copy link
Member

Hey @mjiruobe 👋 No stress on the merge conflicts. I can get to those! They're coming from the combined modal base component that got sent through for #582. Make whatever changes you want to, and then I'll get to the conflicts after 😊

@andrewtavis
Copy link
Member

Hey @mjiruobe 👋 Let me know what the status of this is. Happy to jump into the code and fix the buttons for you if need be! No stress 😎

Copy link
Member

@andrewtavis andrewtavis left a comment

Choose a reason for hiding this comment

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

Got a bit carried away with fixing this, @mjiruobe, but I think we're in dramatically a better place for it all 😊 I realized that your implementation - though 100% sensical given the current architecture - really made it clear that we were relying on BtnLabeled to do too many things and that it was going to start getting even more difficult to maintain.

I thus made #642 to split BtnLabeled so that we could have a separate dropdown button from which this feature could function. The changes for that and all of the i18n key switches that it entailed are included in the most recent commit 🙌

Thanks so much for all your hard work on this issue! Really is functioning so well, was easy for me to make the needed changes, and activist really is so much better off given the new additions that this process made clear!

I'll write a bit about all this in the dev channel on Matrix now so people understand the changes :) :)

@andrewtavis andrewtavis merged commit 94735a0 into activist-org:main Jan 6, 2024
7 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.

2 participants