-
Notifications
You must be signed in to change notification settings - Fork 210
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
Align box to center in mobile view #502
Conversation
* Add manifest.json * cache static assets for offline use * update cache * add meta theme-color and change static files to be cache * cache the files on network request * caching on first run Signed-off-by: tech4GT <varun.gupta1798@gmail.com> * add a button to clear cache * add styling to clear cache link
I've arranged the modules in alphabetical order.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try removing that line, there'll be no need for other changes I guess...
@@ -137,8 +137,9 @@ h1 { | |||
} | |||
#dwnld { | |||
max-width: 200px; | |||
margin: 20px auto; | |||
margin-left: 5px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JonathanXu1 Only removing this would push it in the center...have you tried removing margin-left: 5px
? I guess there is no other change required...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes there's the proper margins on line 140-142.
image-sequencer/examples/demo.css
Lines 138 to 143 in 917831e
#dwnld { | |
max-width: 200px; | |
margin-left: 5px; | |
margin-right: 5px; | |
margin-top: 20px | |
} |
@JonathanXu1 just checking if you need any help. Thanks! |
I submitted this a while back, but it seems that it wasn't merged at the time since the issue was reserved as a GCI beginner task. I think the PR has no issues, thanks! |
@JonathanXu1 would you like to give a try to @Rishabh570 suggestion - #502 (comment) ? thanks! |
@gauravano @JonathanXu1 many changes have been made in #554 I think Rishab's suggestion has been implemented too! |
Hi! If you could rebase and upload a screenshot we can merge this! Or if you think it's been mostly implemented in other PRs, we can close it. Thanks for your work on this, everyone! |
This PR is redundant due to recent changes. Can this one be closed @jywarren @gauravano? |
Seems like this is being fixed in #669. Also the author for this one doesn't seem to be working on this. I think this can be closed. |
This issue seems like a duplicate ofhttps://github.com//pull/669. Closing this for now. @JonathanXu1 please feel free to reopen this if needed. Thanks for your contribution. |
Fixes #484
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
rake test
@publiclab/reviewers
for help, in a comment belowThe dwnld div is not center aligned on mobile view:
On desktop view it is still in the same position:
This task isn't available on GCI, so while I'm waiting for it to be made, I'll be working on #247 . @publiclab/reviewers please let me know if there's anything you need :)