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

Quests Table #6808

Merged
merged 13 commits into from
Jun 25, 2020
Merged

Quests Table #6808

merged 13 commits into from
Jun 25, 2020

Conversation

molecula451
Copy link
Contributor

@molecula451 molecula451 commented Jun 9, 2020

Description

#6718

Tables in Quest

Screenshot from 2020-06-12 11-10-35

Refers/Fixes
Testing

@molecula451
Copy link
Contributor Author

Because I love the quest card style commented them out until further decision from the core-team. We cannot rely on the SVG loading and it's expensive in performance for the server as of now

@molecula451
Copy link
Contributor Author

@thelostone-mc

@codecov
Copy link

codecov bot commented Jun 9, 2020

Codecov Report

Merging #6808 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6808      +/-   ##
==========================================
+ Coverage   26.93%   26.95%   +0.02%     
==========================================
  Files         297      297              
  Lines       28373    28373              
  Branches     4153     4153              
==========================================
+ Hits         7641     7647       +6     
+ Misses      20458    20452       -6     
  Partials      274      274              
Impacted Files Coverage Δ
app/dashboard/embed.py 31.60% <0.00%> (+3.44%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 26fc384...940e8ae. Read the comment docs.

@owocki
Copy link
Contributor

owocki commented Jun 9, 2020

https://bits.owocki.com/Z4uYJp8d

just pulled this down, why are we using the space on the landing page so aggressively? IMHO there should only be one header row, we should move the "Play Quest" and "Play Time" elements to the rows, and reduce the padding between rows

@molecula451
Copy link
Contributor Author

https://bits.owocki.com/Z4uYJp8d

just pulled this down, why are we using the space on the landing page so aggressively? IMHO there should only be one header row, we should move the "Play Quest" and "Play Time" elements to the rows, and reduce the padding between rows

Working things up! this one looks cleaner. I still need to add some layout data properties to make it ready. Please note the I've changed the "Play Quest" button with a Gamepad font icon

quests

@owocki
Copy link
Contributor

owocki commented Jun 11, 2020 via email

@molecula451
Copy link
Contributor Author

looking much better. i'd lose the glow on the header row. also lets make the columns align pls.

This is what is coming up!!

quests

When in "Created" tab an "Ownership" row appear with link to edit quest
Quest change stage on Play row (cooldown, beaten, and unlocked to play)

@molecula451 molecula451 marked this pull request as ready for review June 15, 2020 18:12
@thelostone-mc
Copy link
Member

Code looks alright !
@molecula451 could you remove the commented out code 🙌 should be lgtm otherwise

@molecula451
Copy link
Contributor Author

Because I love the quest card style commented them out until further decision from the core-team. We cannot rely on the SVG loading and it's expensive in performance for the server as of now

@thelostone-mc Please refer the above. Unsure which decision you guys will take for the card style. Hiding them on toggle is probably an idea

@owocki
Copy link
Contributor

owocki commented Jun 18, 2020

i agree with removing the dead(commented) code. otherwise screenshots LGTM

@molecula451 molecula451 changed the title [WIP] Quests Table Quests Table Jun 20, 2020
@molecula451
Copy link
Contributor Author

i agree with removing the dead(commented) code. otherwise screenshots LGTM

Purged dead code!

@molecula451
Copy link
Contributor Author

Beaten quests
beaten

This is pretty much work done gentlemen

@molecula451
Copy link
Contributor Author

purged card CSS

Copy link
Contributor

@PixelantDesign PixelantDesign left a comment

Choose a reason for hiding this comment

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

Is there a way to see what the quest is about before user clicks play?
showing the kudos a user would earn is exicitiing, I wonder if there is a way to work that in maybe on hover somewhere.

@molecula451
Copy link
Contributor Author

Is there a way to see what the quest is about before user clicks play?
showing the kudos a user would earn is exicitiing, I wonder if there is a way to work that in maybe on hover somewhere.

Quest Name
Quest Prize

There is also a Description row

We cannot rely on the SVG. All SVG Are different in properties and it consumes a lot of the performance. Making the section inaccessible

@thelostone-mc thelostone-mc merged commit 0e4eeb3 into gitcoinco:master Jun 25, 2020
@owocki
Copy link
Contributor

owocki commented Jun 29, 2020

@molecula451

checkout this screenshot: https://bits.owocki.com/xQuDqgY9

would you mind doing a follow up PR for a 0.25 ETH that

  1. makes this mobile responsive
  2. allows the user to hover a row (not just the play icon) and then click the whole row ?

@molecula451
Copy link
Contributor Author

@molecula451

checkout this screenshot: https://bits.owocki.com/xQuDqgY9

would you mind doing a follow up PR for a 0.25 ETH that

  1. makes this mobile responsive
  2. allows the user to hover a row (not just the play icon) and then click the whole row ?

Sure thing @owocki will work things up today!

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.

5 participants