woocommerce / sensei-certificates

Hi, I'm the Certificates extension for Sensei.
GNU General Public License v2.0
34 stars 23 forks source link

Add View Certificate Link to Course List templates #327

Closed alexsanford closed 1 year ago

alexsanford commented 1 year ago

Part of https://github.com/Automattic/sensei/issues/6292

Also see https://github.com/Automattic/sensei/pull/6296

Changes proposed in this Pull Request

Add "View Certificate" button to the Course List block templates.

Notes

This was trickier than it seemed at first. In order to add the View Certificate button through a filter, we need to:

This is a bit ugly tbh, but I'm not sure there a better way to do it. There are some implications of doing it like this:

Testing instructions

Design notes

cc @Luchadores

Screenshot 2022-12-14 at 16 44 35
Screenshot 2022-12-14 at 16 44 06
Luchadores commented 1 year ago

Hey Alex 👋

First of all, this is nice! I wonder if View Certificate should be a primary CTA at all. Now we would have two buttons with the same importance on one card, and that doesn't look right.

Question: Is it essential to have "Show Certificate" right in the Course List block? What happens when I click View Results? Where does it send the user? Maybe we can have the "Show Certificate" there? I wanted to tag Ronnie also on this, but for some reason, I cannot find his handle.

image

Lastly, I would make sure the buttons align at the bottom perfectly in the row. What I mean by that is that I would align "Start Course" with "View Certificate" in all the columns.

image
burtrw commented 1 year ago

I think I asked for it, and I would prefer to link to it directly from the block :)

Aligning the buttons is an ongoing issue, though. What about if the course is complete we change to text links for Visit Results and View Certificate?

burtrw commented 1 year ago

Maybe like we have the 'Course Overview' link option?

alexsanford commented 1 year ago

@Luchadores I can definitely align the buttons to the bottom to fix that issue.

What are your thoughts on changing them to links instead of buttons as @burtrw suggested? Alternatively, I could change the certificate button to use the "outline" style instead of "fill" (see below for how this would look in Twenty Twenty-Three). Would that work?

Screenshot 2022-12-15 at 11 20 13
Luchadores commented 1 year ago

What do you both think about this? @burtrw

Having "View Certificate" as a link under the Course Overview makes more sense to me.

image

burtrw commented 1 year ago

I like matching the Course Overview link - too many themes probably won't have the outline button style :(

jom commented 1 year ago

Hi Alex! Just checking some things.

alexsanford commented 1 year ago

@Luchadores @burtrw I've changed the design in the latest iteration to use a link, inserted under the "Course Overview" link. Here's how it looks now in my dev environment (note that "Course 1" is the only one that has a certificate):

Grid

Screenshot 2022-12-16 at 11 41 52

List

Screenshot 2022-12-16 at 12 02 26
alexsanford commented 1 year ago

@jom

If I add the course list block before activating Certificates, when I do activate certificates the View Certificates button doesn't show up.

That's correct. It's because the template is applied when the block is first added, and so we don't actually add the block. The user would have to remove the Course List block and re-add it.

If I click inside to add the View Certificates button manually, on the frontend after it shows up twice for completed course (did it partially fix on save?). ... If I then replace the block with the pattern again (with Certificates active), it works as currently designed.

Could you test again? I've changed how the link is added, so this behaviour should be fixed. I just tested and can't reproduce.