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

Styling the Forum Vote Buttons #388

Merged
merged 20 commits into from
Dec 6, 2023
Merged

Styling the Forum Vote Buttons #388

merged 20 commits into from
Dec 6, 2023

Conversation

giomhern
Copy link
Contributor

@giomhern giomhern commented Dec 3, 2023

Purpose

Fix the rather clunky looking visuals of the forum like and dislike buttons. The intended result is to have buttons that fit the theme of the machina forum.

@giomhern giomhern linked an issue Dec 3, 2023 that may be closed by this pull request
4 tasks
@giomhern giomhern requested a review from bburgess19 December 3, 2023 03:50
@giomhern giomhern self-assigned this Dec 3, 2023
@giomhern giomhern added this to the 2023/Sprint 3 milestone Dec 3, 2023
@giomhern giomhern requested a review from majorsylvie December 3, 2023 03:51
Copy link
Contributor

@bburgess19 bburgess19 left a comment

Choose a reason for hiding this comment

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

Review

Great work overall!

I like the placement of the buttons, and it looks great even on large bodies of text:
image

Suggestion

Let me know what you think about adding a larger gap between the items in the flex box.

Here's the current gap-1 on the .vote-container:
image

and here's gap-3:
image

src/templates/forum_conversation/topic_detail.html Outdated Show resolved Hide resolved
src/templates/forum_conversation/topic_detail.html Outdated Show resolved Hide resolved
src/templates/forum_conversation/topic_detail.html Outdated Show resolved Hide resolved
Copy link
Contributor

@bburgess19 bburgess19 left a comment

Choose a reason for hiding this comment

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

These are changes bringing machina changes into our templates. The code and the buttons looks good to me!

@giomhern giomhern force-pushed the forums/vote-buttons-css branch from 4f029a4 to 675ba5f Compare December 5, 2023 04:04
@giomhern giomhern closed this Dec 5, 2023
@giomhern giomhern force-pushed the forums/vote-buttons-css branch from 675ba5f to 4e1f6c9 Compare December 5, 2023 04:16
@majorsylvie majorsylvie reopened this Dec 5, 2023
@giomhern giomhern requested a review from bburgess19 December 5, 2023 18:24
Copy link
Contributor

@bburgess19 bburgess19 left a comment

Choose a reason for hiding this comment

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

Report

I like the stylings! Just implement the styles based on the user vote then resubmit!

value="like">
<i class="fas fa-thumbs-up"></i> Like
</button>
<span class="like-count fas">{{ post.rating }}</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

Color like button text

As noted by @joshuamonroe mentioned here, we should have some color indicator based the user's vote.

e.g. the user disliked:
image

the user liked:
image

The colors used here are the same colors from the buttons:

#3579F6 // blue
#CB444A // red

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have instead just changed the color of the icon as opposed to the number because I figured it would be somewhat convulated for sake of people like me who are colorblind LOL. Attached are photos of the changes instead.
Screenshot 2023-12-05 at 2 55 09 PM
Screenshot 2023-12-05 at 2 55 15 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To add onto this, the deselecting when the other one is selected is not completely working. It works from like --> unlike but not the opposite way, but this can be a minor PR.

@giomhern giomhern requested a review from bburgess19 December 5, 2023 21:05
Copy link
Contributor

@majorsylvie majorsylvie left a comment

Choose a reason for hiding this comment

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

Running on firefox I do not see the graphics:

image image

Even after force refreshing the cache (mac: cmd + shift + r), the thumbs up/down don't appear.

would be good to have this working in a way that is supported on other/older browsers!

@majorsylvie majorsylvie self-requested a review December 6, 2023 00:41
@bburgess19 bburgess19 removed their request for review December 6, 2023 00:42
Copy link
Contributor

@majorsylvie majorsylvie left a comment

Choose a reason for hiding this comment

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

Looks better!

FYI, the placement of the buttons shifts depending on the width of the current vote count:

Screen.Recording.2023-12-05.at.6.41.00.PM.mov

This is not a problem for this PR, something you might want to make a separate issue if this behaviour is not desired.

@joshuamonroe joshuamonroe self-requested a review December 6, 2023 01:05
Copy link
Contributor

@joshuamonroe joshuamonroe left a comment

Choose a reason for hiding this comment

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

This looks great and everything seems to be working as intended on my end. One very small housekeeping request in the code comments and then I'll be happy to approve it.

@@ -19,7 +19,7 @@
<link rel="stylesheet" href="{% static 'machina/custom-styling.css' %}" />
{% endblock css %}
{% block body %}
<div class="my-5 container" id="main_container">
<div class="my-5 container " id="main_container">
Copy link
Contributor

Choose a reason for hiding this comment

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

Very minor, but lets get rid of this extra space to keep the PR clean.

@bburgess19 bburgess19 force-pushed the forums/vote-buttons-css branch 2 times, most recently from c1d83ff to d2968ce Compare December 6, 2023 01:24
@bburgess19 bburgess19 force-pushed the forums/vote-buttons-css branch from d2968ce to 6b50ffc Compare December 6, 2023 01:26
@joshuamonroe joshuamonroe self-requested a review December 6, 2023 01:27
Copy link
Contributor

@joshuamonroe joshuamonroe left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning that up, good to go now. Approved!

@majorsylvie majorsylvie requested review from bburgess19 and removed request for bburgess19 December 6, 2023 01:30
@bburgess19 bburgess19 removed their request for review December 6, 2023 01:31
Copy link
Contributor

@bburgess19 bburgess19 left a comment

Choose a reason for hiding this comment

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

The buttons look great to me! I added a min-width to the buttons at the end to make sure changing the vote count doesn't move the buttons around until the vote count is ridiculously large (>100,000)

@majorsylvie majorsylvie merged commit b1fe3f7 into dev Dec 6, 2023
2 checks passed
@majorsylvie majorsylvie deleted the forums/vote-buttons-css branch December 6, 2023 01:33
@majorsylvie majorsylvie assigned bburgess19 and unassigned bburgess19 Dec 7, 2023
@bburgess19
Copy link
Contributor

Closing comment

We added styles to the barebones like buttons, and ensured they behaved properly when toggling and switching between button clicks.

Detailed notes

We faced some issues with the voting icons not appearing for Firefox, but that was solved by using FA5 class names for those icons, i.e. fas instead of fa-solid. (credits: @majorsylvie)

We made sure the user's vote was clear by choosing to fill an icon instead of solely changing the icon's color. This helped with accessibility. (credits: @giomhern )

We added a bit of extra space around the vote counter so the button placement would shift so much with every vote. (credits: @majorsylvie)

Thank you to @joshuamonroe and @majorsylvie for reviewing, and @giomhern for spear-heading this!

@majorsylvie
Copy link
Contributor

Issue Score: Excellent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Fix CSS for Like and Dislike Buttons in Forums
4 participants