-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Fix #1800: remove unnecessary margins and update responsive breakpoint #2418
Conversation
@@ -67,3 +67,4 @@ The Oppia code is released under the [Apache v2 license](https://github.com/oppi | |||
|
|||
We also have public chat rooms on Gitter: [https://gitter.im/oppia/oppia-chat](https://gitter.im/oppia/oppia-chat) and the #oppia channel on Freenode IRC. Drop by and say hello! | |||
|
|||
# learnPhaserZenva |
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.
Hi @anthkris ... what is this?
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.
Holy crap! That's what I get for working on multiple projects I guess. I'll fix it.
Current coverage is 47.76% (diff: 100%)@@ develop #2418 diff @@
==========================================
Files 191 191
Lines 16151 16152 +1
Methods 2788 2788
Messages 0 0
Branches 2741 2741
==========================================
+ Hits 7714 7715 +1
Misses 8437 8437
Partials 0 0
|
@@ -2746,10 +2746,6 @@ div#ng-curtain { | |||
margin-bottom: 10px; | |||
} | |||
|
|||
.oppia-notifications-tray { |
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.
Is this CSS class being used anywhere else? If not, delete it from the html file too?
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.
Done.
I still can't run e2e tests, even with the new version. I get the same errors as always. Also, tried to restart the test per the new instructions and Travis said it couldn't restart them. |
I've restarted the test. Not sure what you mean by the new version? What's the error you get from travis when trying to restart the tests? |
Hi @anthkris, thanks! I have no further concerns about the code. @souravbadami, would you mind taking this PR for a spin and seeing if it addresses the concerns you raised in #1800? Thanks! |
Thanks @wxyxinyu I meant the new Oppia version. I had to redownload everything because I couldn't get it to work. But e2e tests have never worked for me and still aren't working. Travis gave me an error (I don't remember the exact language) but it basically said that it couldn't restart the test and then I tried on the main page and it said it couldn't restart the build. That was it. |
Hm, weird. If the Travis thing happens again, would you mind taking a screenshot? (I seem to remember seeing an error like that before, which was due to Travis being flaky, but I'm not sure if this is the same one.) Alas, I'm still not sure what to do about the e2e tests not running on your machine, though... |
Hi @souravbadami ... ping? |
@seanlip -- LGTM. |
Most of this work seems to have been done for the last major release. I did some minor tweaks to update the breakpoint where margins were applied and removed some unnecessary margins that made the headings out of alignment with the card.