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

improve toggle sidebar #451

Merged
merged 11 commits into from
May 7, 2019
Merged

improve toggle sidebar #451

merged 11 commits into from
May 7, 2019

Conversation

avsingh999
Copy link
Member

@avsingh999 avsingh999 commented Mar 25, 2019

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

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

  • tests pass -- rake test
  • code has been rebased on top of latest master (check if another pull request was added recently, and please rebase)
  • pull request is descriptively named and, if possible, multiple commits squashed if they're smaller changes

Thanks!

@avsingh999
Copy link
Member Author

@jywarren @SidharthBansal @gauravano please review
pub11

@jywarren
Copy link
Member

Oops, can you remove passenger.3001.pid and passenger.3001.pid.lock? Then this is good to merge, thanks so much!!!

@avsingh999
Copy link
Member Author

@jywarren @gauravano @SidharthBansal please review me.
thanks ; )

@grvsachdeva
Copy link
Member

Hi @avsingh999, please remove the merge conflicts here. Thanks!

@codeclimate
Copy link

codeclimate bot commented Apr 30, 2019

Code Climate has analyzed commit 9c8ee86 and detected 0 issues on this pull request.

View more on Code Climate.

@sashadev-sky
Copy link
Member

sashadev-sky commented Apr 30, 2019

@avsingh999 @gauravano @jywarren I pulled this implementation and it was not working for me. @avsingh999 Perhaps your rebase might have pulled in some changes that were not compatible with your code. I didn't look into this too much because I figured there was a much simpler way to go about this anyway, and I committed that here.

@avsingh999 let me know if this is fine with you! Always happy to revert if you have any objections. -- sorry it wouldn't let me add you as a reviewer for some reason but would like your review on this pls :)

@codecov
Copy link

codecov bot commented Apr 30, 2019

Codecov Report

❗ No coverage uploaded for pull request base (main@9b09a9b). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #451   +/-   ##
=======================================
  Coverage        ?   70.07%           
=======================================
  Files           ?       31           
  Lines           ?     1243           
  Branches        ?        0           
=======================================
  Hits            ?      871           
  Misses          ?      372           
  Partials        ?        0

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 9b09a9b...9c8ee86. Read the comment docs.

@sashadev-sky
Copy link
Member

@avsingh999 @gauravano @jywarren my view after this rebase and my commit looks just like the one @avsingh999 posted a gif of above.

Before my commit this is what I was seeing:
chevron-toggle

@jywarren jywarren merged commit 8ed7283 into publiclab:main May 7, 2019
@jywarren
Copy link
Member

jywarren commented May 7, 2019

👍

chen-robert pushed a commit to chen-robert/mapknitter that referenced this pull request Dec 5, 2019
* Show only maps with at least one image in main listing

* Update Leaflet.DistortableImage version

* improve setting dropdown

* improve toggle sidebar

* Delete passenger.3001.pid

* Delete passenger.3001.pid.lock

* Update map.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

change icon when toggle side bar
4 participants