Skip to content

Add donate dialog#79

Merged
ArsalaBangash merged 14 commits intomasterfrom
add-donate-dialog
Nov 7, 2020
Merged

Add donate dialog#79
ArsalaBangash merged 14 commits intomasterfrom
add-donate-dialog

Conversation

@dlqqq
Copy link
Contributor

@dlqqq dlqqq commented Oct 28, 2020

Description

This PR adds a donation dialog, with separate PayPal, OC, and GH donation icons & buttons. I've also had to put the 28x28 minified icons in the src/statics directory, since I couldn't find any free CDN distributing these icons.

I also got rid of the old HTML/CSS, and switched to using the default QBtn template provided by Quasar. It looks a lot cleaner than the nested <a><button></button></a> old HTML. Example:

<q-btn
              class="q-pa-xs q-ma-sm"
              icon="img:../statics/icons/paypal-28x28.png"
              label="Donate via Paypal"
              type="a"
              href="https://www.paypal.com/cgi-bin/webscr?cmd=_s-xclick&hosted_button_id=VEAGAZP7DHJNE&source=url"
              target="_blank"
/>

Showcase

20201028-124620

20201028-124629

@dlqqq dlqqq requested a review from ArsalaBangash October 28, 2020 19:50
@ArsalaBangash
Copy link
Member

@diracs-delta The build failed :(

12:52:38 AM: ERROR in /opt/build/repo/packages/app/src/components/DonationDialog.vue
12:52:38 AM: ERROR in /opt/build/repo/packages/app/src/components/DonationDialog.vue(54,12):
12:52:38 AM: TS2339: Property 'inDonation' does not exist on type '{ open(): void; }'.
12:52:38 AM:  app:build [FAIL] Build failed with errors. Check log above. +0ms

@dlqqq
Copy link
Contributor Author

dlqqq commented Oct 28, 2020

hm. the offending code segment seems to be the script in DonationDialog.vue.

<script lang="ts">
export default {
  name: 'donation-dialog',
  data () {
    return {
      inDonation: false,
    }
  },
  methods: {
    open () {
      this.inDonation = true
    },
  },
}
</script>

For some reason, it looks like TS is recognizing this as an object of type {open: () => void} instead of the Vue instance.

@ArsalaBangash
Copy link
Member

I think I remember this happening elsewhere. Let me find other uses of this

@ArsalaBangash
Copy link
Member

@diracs-delta This should fix it. Quasar is currently working on better TS support

isTimed() {
      const app: any = this;
      return app.practiceMode === PracticeMode.TIME;
 },

@dlqqq
Copy link
Contributor Author

dlqqq commented Oct 28, 2020

pushing another commit right now. think this will fix it, since the build completes on my end.

@dlqqq
Copy link
Contributor Author

dlqqq commented Oct 28, 2020

i believe it's bad practice to cast types as any just to circumvent TS compiler errors; that kind of defeats the purpose of type-checking if we disable its functionality every time we run into a problem. i read this article here:

https://v3.vuejs.org/guide/typescript-support.html#using-with-options-api

which talks about how the TS compiler has difficulty inferring types in the Vue framework. Explicitly specifying them with an interface seems to help.

@dlqqq
Copy link
Contributor Author

dlqqq commented Oct 28, 2020

huzzah. all checks passed. 👍

finally i can get some lunch

@ArsalaBangash
Copy link
Member

i believe it's bad practice to cast types as any just to circumvent TS compiler errors; that kind of defeats the purpose of type-checking if we disable its functionality every time we run into a problem. i read this article here:

https://v3.vuejs.org/guide/typescript-support.html#using-with-options-api

which talks about how the TS compiler has difficulty inferring types in the Vue framework. Explicitly specifying them with an interface seems to help.

This is excellent. We should refactor all instances of this anti-pattern to use interfaces :)

Copy link
Member

@ArsalaBangash ArsalaBangash left a comment

Choose a reason for hiding this comment

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

Maybe we should consider using a dropdown button instead: https://quasar.dev/vue-components/button-dropdown

What do you thinK?

<div class="column items-center">
<q-btn
class="q-pa-xs q-ma-sm"
icon="img:../statics/icons/paypal-28x28.png"
Copy link
Member

Choose a reason for hiding this comment

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

Please use the mdi-paypal icon

Copy link
Contributor Author

Choose a reason for hiding this comment

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

uh... there isn't a native mdi-paypal icon that ships with quasar. the only mention of one i could find is here

https://themesdesign.in/xoric/layouts/blue/vertical/icons-materialdesign.html

but this isn't the mdi icon set quasar ships with.

Copy link
Member

Choose a reason for hiding this comment

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

@dlqqq
Copy link
Contributor Author

dlqqq commented Oct 28, 2020

Maybe we should consider using a dropdown button instead: https://quasar.dev/vue-components/button-dropdown

What do you thinK?

@ArsalaBangash I don't know. I think it's a unique solution that achieves more sleek UI, but the downside is that there's no cute message thanking supporters who click and consider donating. I would leave it as it is right now, but it's your call.

@ArsalaBangash
Copy link
Member

Maybe we should consider using a dropdown button instead: https://quasar.dev/vue-components/button-dropdown
What do you thinK?

@ArsalaBangash I don't know. I think it's a unique solution that achieves more sleek UI, but the downside is that there's no cute message thanking supporters who click and consider donating. I would leave it as it is right now, but it's your call.

I agree with you about the messaging. Let's add the following:

This app was developed by Grey Software, a a not-for-profit organization that empowers students to build open-source software for their communities and societies.

You can support us as we envision and build the software ecosystem of the future by sponsoring us on GitHub backing us on OpenCollective, or making a donation via PayPal.

Copy link
Member

@ArsalaBangash ArsalaBangash left a comment

Choose a reason for hiding this comment

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

Solid Iteration!

Great work. I have a couple suggestions:

@dlqqq
Copy link
Contributor Author

dlqqq commented Nov 5, 2020

Since we're using a q-card component, we could consider using the card action UI pattern for our donate buttons: https://quasar.dev/vue-components/card#With-actions

added. this is the current layout:

20201105-130555

I think it would be good for the org to use a standard set of resources, so I suggest using the icons from the website: https://github.com/grey-software/grey.software/tree/master/assets/icons

we'll do that and use the SVG icons later once #83 is merged, so we can use vue-svg-loader for that. i've set up a separate issue to remind ourselves here: #84

@ArsalaBangash
Copy link
Member

I made a few fixes to the layout:

I put the label and the donate button in a row

image

Added some spacing and left-alignment to the DonateDialog

image

Copy link
Member

@ArsalaBangash ArsalaBangash left a comment

Choose a reason for hiding this comment

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

Excellent PR

@ArsalaBangash ArsalaBangash merged commit d4e5ca9 into master Nov 7, 2020
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