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

Some new Fixes added for Quests on Mobile UI #5463

Merged
merged 4 commits into from
Nov 12, 2019
Merged

Some new Fixes added for Quests on Mobile UI #5463

merged 4 commits into from
Nov 12, 2019

Conversation

molecula451
Copy link
Contributor

@molecula451 molecula451 commented Nov 7, 2019

Description

We recently found that our style sheets in our Quests code were a tiny bit unsorted for our Mobile UI/UX.

Quest Start on Mobile UI looked Akward:
emcharacter

Character Class Unsorted and unresponsive
character

Refers/Fixes

Unable to click submit -metacartel quest mobile UI 5418

    • Added Comments on new Style Rules for better Code Maintenance
    • Added New Style Rule (@ media) for Better Mobile UI (500px)
    • Character Class unsorted on stage2 on Mobile UI aligned accordingly to the right border
    • Button Line-Height px unresponsive on iPhone (fixed)
    • Quests Rules fixed for better alignment on the iPhone

Character should look Now:

fixedcharacter

Quest Rules Style for Mobile UI was extremely bigger. We made some fixes so it looks like

quest rules

User was claiming that he actually could not submit it's answers because button was hitting the other element
button1

  • User should be able to submit it's answer on any stage.

There are still many elements that are not well sorted on the site and that they need to be fix. I will be providing updates on a constantly basis aswell as on style document.

Guys also. If am leaving something left behind that I am not being able to look please let me know and I will take care

Testing
  • Brave Browser
  • VSCode
  • Web3
  • Gitcoin

@molecula451 molecula451 marked this pull request as ready for review November 7, 2019 02:44
Copy link
Member

@thelostone-mc thelostone-mc left a comment

Choose a reason for hiding this comment

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

Left a few comments!
Also as opposed to using margin-top / position: absolute

Could we use boostrap classes which are already available

like
<div class="position-abosolute">
<div class="mt-2 mt-sm-0"> and so on

Copy link
Contributor Author

@molecula451 molecula451 left a comment

Choose a reason for hiding this comment

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

Left a few comments!
Also as opposed to using margin-top / position: absolute

Could we use boostrap classes which are already available

like
<div class="position-abosolute">
<div class="mt-2 mt-sm-0"> and so on

Quick positioning classes are available, though they are not responsive.

@thelostone-mc If you want we can add it. But it may mess the whole code maintenance a long the way

app/assets/v2/css/quests.css Outdated Show resolved Hide resolved
Added New Formated Document for better readability
Curly Braces identation was fixed in whole code
@codecov
Copy link

codecov bot commented Nov 11, 2019

Codecov Report

Merging #5463 into master will decrease coverage by 0.49%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #5463     +/-   ##
=========================================
- Coverage   29.64%   29.15%   -0.5%     
=========================================
  Files         242      242             
  Lines       20811    20849     +38     
  Branches     3008     3016      +8     
=========================================
- Hits         6169     6078     -91     
- Misses      14387    14526    +139     
+ Partials      255      245     -10
Impacted Files Coverage Δ
app/kudos/test_views.py 0% <0%> (-100%) ⬇️
app/kudos/test_models.py 0% <0%> (-100%) ⬇️
app/kudos/test_utils.py 0% <0%> (-70.59%) ⬇️
app/app/context.py 0% <0%> (-48.15%) ⬇️
app/retail/helpers.py 28.57% <0%> (-42.86%) ⬇️
app/retail/templatetags/matches.py 62.5% <0%> (-37.5%) ⬇️
app/retail/templatetags/is_in_list.py 66.66% <0%> (-33.34%) ⬇️
app/kudos/views.py 15.63% <0%> (-6.47%) ⬇️
app/quests/views.py 15.04% <0%> (-0.85%) ⬇️
app/kudos/models.py 52.44% <0%> (-0.7%) ⬇️
... and 6 more

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 71d5ff3...ae79481. Read the comment docs.

Copy link
Contributor Author

@molecula451 molecula451 left a comment

Choose a reason for hiding this comment

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

Now that I am able to play with the whole document

  • Curly Braces Identation fixed in whole document

  • New Document Formatted for Better Code Readability

  • Added New Rules and queries for different screen ranges

  • On the stage 4 the quest answer could not be possible submitted. Linge-height of the .button class was fixed so user it's able to send it's answers Unable to click submit -metacartel quest mobile UI #5418

  • More comments for better code maintenance

@molecula451
Copy link
Contributor Author

molecula451 commented Nov 11, 2019

I will keep adding fixes and provide updates on this specific code so our quests are always fully responsive and intuitive.

If I am leaving something left behind. Please let me know in advance

@@ -1555,7 +1555,7 @@
}
body.stage_4 #header {
margin-top: -2%;
font-size: 12px;
font-size: 8px;
Copy link
Contributor Author

@molecula451 molecula451 Nov 11, 2019

Choose a reason for hiding this comment

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

Font-Size reduction so our user it's able to actually read the Question in it's phone

question

@molecula451
Copy link
Contributor Author

@thelostone-mc expecting some feedback awesome mate

@thelostone-mc
Copy link
Member

@molecula451 I udpated the indentation and stuff cause I wanted to get this in before this weeks deploy ^_^

@thelostone-mc thelostone-mc merged commit 2445d79 into gitcoinco:master Nov 12, 2019
@molecula451
Copy link
Contributor Author

@molecula451 I udpated the indentation and stuff cause I wanted to get this in before this weeks deploy ^_^

I was eager to know if you wanted to keep the 4-5 spaces latest indentation which was not from the original document. But it's all ok. Good stuff great friend

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