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

Feature/claim card votes #594

Merged
merged 8 commits into from
Jul 16, 2021
Merged

Feature/claim card votes #594

merged 8 commits into from
Jul 16, 2021

Conversation

victorgdev
Copy link
Contributor

What issue does this PR close

Closes #559

Changes Proposed ( a list of new changes introduced by this PR)

Implements voting status bar to the claim card

How to test ( a list of instructions on how to test this PR)

  • Go to /dashboard or /dashboard/analysis or profile/<username>/claims
  • See claim cards

@victorgdev victorgdev marked this pull request as draft July 13, 2021 21:06
@victorgdev
Copy link
Contributor Author

@lucca65 need your input, how should we go about the following issues:

  • Claims that have 7 or 9 verifiers have bad text-behavior on the vote type and number.

1626211204284

  • There are some questions about the Figma design wanted to share:
    • Two options for pending votes title (Pending and Left) - Currently using Pending text
    • Gray color of the bar there are two versions (lighter and darker) - Currently using the lighter color
    • Day count of claim was available in one card and not the other (if newClaim then EMPTY else dayCount)? - Yet to be implemented

@henriquecbuss
Copy link
Member

@juramos-2020 might want to see this as well ☝️

@juramos-2020
Copy link

Hi @victorgdev

I updated de figma

Two options for pending votes title (Pending and Left) - Currently using Pending text

You can use "Pending"

Gray color of the bar there are two versions (lighter and darker) - Currently using the lighter color

I correct the shade of gray that was in a bar. I'am using one gray colo to progress bar and other to text.

Day count of claim was available in one card and not the other (if newClaim then EMPTY else dayCount)? - Yet to be implemented

I updated too. 😉👍

@victorgdev
Copy link
Contributor Author

victorgdev commented Jul 14, 2021

@juramos-2020 we still have one problem to solve, which is for claims that have 7 or 9 verifiers (first and second cards in the photo below) there is bad positioning behavior on the vote type and number of votes. Can you help us on how to go about it?

1626211204284

@lucca65
Copy link
Member

lucca65 commented Jul 14, 2021

i would always hide the votes and show the text only on hover or click on the mobile

keep the numbers tho

the colors are super self explanatory

@victorgdev victorgdev marked this pull request as ready for review July 15, 2021 14:31
henriquecbuss
henriquecbuss previously approved these changes Jul 15, 2021
Copy link
Member

@henriquecbuss henriquecbuss left a comment

Choose a reason for hiding this comment

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

📊

src/elm/Claim.elm Outdated Show resolved Hide resolved
src/elm/Claim.elm Outdated Show resolved Hide resolved
src/elm/Claim.elm Outdated Show resolved Hide resolved
src/elm/Claim.elm Outdated Show resolved Hide resolved
src/elm/Page/Dashboard/Claim.elm Outdated Show resolved Hide resolved
@victorgdev victorgdev requested review from juramos-2020 and removed request for juramos-2020 July 15, 2021 16:49
Copy link
Member

@lucca65 lucca65 left a comment

Choose a reason for hiding this comment

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

bro can you change one translation, please?

There is a pt-BR error here:

Captura de Tela 2021-07-15 às 14 49 29

the correct one is "Analisadas"

Its also looking weird on the modal, check this out:

Captura de Tela 2021-07-15 às 14 51 34

It would be nice to have a little margin button after the content of the modal is done:

Captura de Tela 2021-07-15 às 14 52 05

And lastly, there is one thing that is bothering me... when the claim is approved, we no longer have the tag to say if it was rejected, approved. We still have the filter for "pending" but still its weird to read a "PENDING" text and the claim was actually approved.

What do you folks think about us adding a full bar when its completed? can be full green and the pending can keep disappear

@victorgdev @juramos-2020 lets do a quick call if needed to discuss this, I think we are, again, running in circles

Captura de Tela 2021-07-15 às 14 55 47

@juramos-2020 juramos-2020 self-assigned this Jul 15, 2021
@victorgdev
Copy link
Contributor Author

victorgdev commented Jul 15, 2021

Just for recap, @lucca65 and @juramos-2020 here are the discussed changes to the designs on this feature:

- Voting bar:
IF claim status != pending, then

  • Voting bar will be fully colored, green for approved and red for disapproved status.
  • Claim status name will be placed top right above the voting bar
  • #votes and vote text will placed bottom right below the voting bar

ELSE
Remains as is

- Claim Modal and Detail page:

  • Remove status subtitles

- Last Change Requests:

  • BR Translation change (Analisadas)
  • mb increase on modal content

@victorgdev victorgdev requested a review from lucca65 July 15, 2021 21:15
@juramos-2020
Copy link

Have the changes already been made?

I did a test now and it's all right with the progress bar in the card and modal.

Copy link
Member

@lucca65 lucca65 left a comment

Choose a reason for hiding this comment

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

sweet! 🍬 ✨

@victorgdev
Copy link
Contributor Author

Hi @juramos-2020, yes the requested changes are done.

@victorgdev victorgdev merged commit b74b160 into master Jul 16, 2021
@victorgdev victorgdev deleted the feature/claim-card-votes branch July 16, 2021 08:19
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.

Show number of votes on claims
4 participants