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

Fix #1800: remove unnecessary margins and update responsive breakpoint #2418

Merged
merged 6 commits into from
Aug 27, 2016

Conversation

anthkris
Copy link
Contributor

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.

@@ -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
Copy link
Member

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?

Copy link
Contributor Author

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.

@codecov-io
Copy link

codecov-io commented Aug 23, 2016

Current coverage is 47.76% (diff: 100%)

Merging #2418 into develop will increase coverage by <.01%

@@            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          

Powered by Codecov. Last update 3752a60...8678025

@@ -2746,10 +2746,6 @@ div#ng-curtain {
margin-bottom: 10px;
}

.oppia-notifications-tray {
Copy link
Member

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@anthkris
Copy link
Contributor Author

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.

@wxyxinyu
Copy link
Contributor

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?

@seanlip
Copy link
Member

seanlip commented Aug 24, 2016

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!

@seanlip seanlip assigned souravbadami and unassigned wxyxinyu Aug 24, 2016
@anthkris
Copy link
Contributor Author

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.

@seanlip
Copy link
Member

seanlip commented Aug 24, 2016

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...

@seanlip
Copy link
Member

seanlip commented Aug 27, 2016

Hi @souravbadami ... ping?

@souravbadami
Copy link
Contributor

@seanlip -- LGTM.

@souravbadami souravbadami merged commit eaac650 into oppia:develop Aug 27, 2016
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.

5 participants