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.

Sign up for GitHub

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

Jump to bottom

Header line removed and Menu icon made more visible for mobile view #7319

Closed
wants to merge 6 commits into from
Closed

Header line removed and Menu icon made more visible for mobile view #7319

wants to merge 6 commits into from

Conversation

NitinBhasneria
Copy link
Collaborator

@NitinBhasneria NitinBhasneria commented Jan 21, 2020 •
edited
Loading

Fixes #7312 (<=== Add issue number here)

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • PR is descriptively titled 📑 and links the original issue above 🔗
  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
  • code is in uniquely-named feature branch and has no merge conflicts 📁
  • screenshots/GIFs are attached 📎 in case of UI updation
  • ask @publiclab/reviewers for help, in a comment below

Screenshot from 2020-01-21 17-30-24
Screenshot from 2020-01-21 17-30-22

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

@codecov
Copy link

codecov bot commented Jan 21, 2020 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7319      +/-   ##
==========================================
+ Coverage   54.98%   55.08%   +0.10%     
==========================================
  Files          97       97              
  Lines        6965     6965              
==========================================
+ Hits         3830     3837       +7     
+ Misses       3135     3128       -7
Impacted Files Coverage Δ
app/services/execute_search.rb 33.33% <0.00%> (+3.03%) ⬆️
app/api/srch/search.rb 56.20% <0.00%> (+3.92%) ⬆️

@NitinBhasneria
Copy link
Collaborator Author

@jywarren @SidharthBansal can you please review this.

@SidharthBansal
Copy link
Member

Can you please fix the travis? It is failing

@Uzay-G
Copy link
Member

Uzay-G commented Jan 30, 2020

The travis is not @NitinBhasneria's fault. I have noticed that sometimes the system tests we have added are failing. Do you have any idea why @VladimirMikulic?

@VladimirMikulic
Copy link
Contributor

VladimirMikulic commented Jan 30, 2020 •
edited
Loading

We have 'wiki revisions' test failing. Logs are showing that "Are you sure" modal didn't show up.
An empty space before the text "Revert"(like this " Revert") or empty <i> could have caused this.

I'll fix it again, in the meantime @NitinBhasneria you can force re-push to restart Travis and you should be good.
@Uzay-G thanks for reporting this.

@VladimirMikulic VladimirMikulic mentioned this pull request Jan 30, 2020
Merged
SidharthBansal pushed a commit that referenced this pull request Jan 31, 2020
@VladimirMikulic
FIX: Wiki revisions test (#7396)
505e13f 
This test has already been fixed before but recently it has failed again
in #7319. The problem might be related to our selection of "Revert"
button.

Inside of the button we have an empty space before the text
and a font-awesome icon(<i>).
We were selecting the button with #row0 and the text of "Revert" which
probably caused the issue.

I've added the btn-revert class to avoid this strange behaviour.
@NitinBhasneria
Copy link
Collaborator Author

Thanks, @VladimirMikulic so should I close this pr and open another one

@VladimirMikulic
Copy link
Contributor

@NitinBhasneria you don't have to close this PR, I've fixed the issue. Just rebase the branch and push again. git rebase master; git push

vinitshahdeo pushed a commit to vinitshahdeo/plots2 that referenced this pull request Feb 1, 2020
@VladimirMikulic
FIX: Wiki revisions test (publiclab#7396)
70a8d64 
This test has already been fixed before but recently it has failed again
in publiclab#7319. The problem might be related to our selection of "Revert"
button.

Inside of the button we have an empty space before the text
and a font-awesome icon(<i>).
We were selecting the button with #row0 and the text of "Revert" which
probably caused the issue.

I've added the btn-revert class to avoid this strange behaviour.
@NitinBhasneria NitinBhasneria requested a review from a team as a code owner February 5, 2020 14:12
@NitinBhasneria
Copy link
Collaborator Author

@VladimirMikulic now also it is showing the error can you please review....

NitinBhasneria pushed a commit to NitinBhasneria/plots2 that referenced this pull request Feb 5, 2020
@VladimirMikulic
FIX: Wiki revisions test (publiclab#7396)
5cef3e1 
This test has already been fixed before but recently it has failed again
in publiclab#7319. The problem might be related to our selection of "Revert"
button.

Inside of the button we have an empty space before the text
and a font-awesome icon(<i>).
We were selecting the button with #row0 and the text of "Revert" which
probably caused the issue.

I've added the btn-revert class to avoid this strange behaviour.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cesswairimu cesswairimu Awaiting requested review from cesswairimu

@ananyaarun ananyaarun Awaiting requested review from ananyaarun

@jywarren jywarren Awaiting requested review from jywarren

@SidharthBansal SidharthBansal Awaiting requested review from SidharthBansal

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Line on header at width 768px
4 participants
@NitinBhasneria @SidharthBansal @Uzay-G @VladimirMikulic