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

Fix #4549 - Fixed the position of code editor #4674

Merged
merged 1 commit into from
Feb 12, 2018
Merged

Fix #4549 - Fixed the position of code editor #4674

merged 1 commit into from
Feb 12, 2018

Conversation

bansalnitish
Copy link
Contributor

@bansalnitish bansalnitish commented Feb 11, 2018 •
edited
Loading

This PR fixes #4549.

Checklist

  • The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes.
  • The linter/Karma presubmit checks have passed.
    • These should run automatically, but if not, you can manually trigger them locally using python scripts/pre_commit_linter.py and bash scripts/run_frontend_tests.sh.
  • The PR is made from a branch that's not called "develop".
  • The PR follows the style guide.
  • The PR is assigned to an appropriate reviewer.
    • If you're a new contributor, please ask on Gitter for someone to assign a reviewer.
    • If you're not sure who the appropriate reviewer is, please assign to the issue's "owner" -- see the "talk-to" label on the issue.

@bansalnitish bansalnitish changed the title Fixes#4549 - Fixed the position of code editor Fix #4549 - Fixed the position of code editor Feb 11, 2018
@codecov-io
Copy link

codecov-io commented Feb 11, 2018 •
edited
Loading

Codecov Report

Merging #4674 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #4674   +/-   ##
========================================
  Coverage    44.88%   44.88%           
========================================
  Files          384      384           
  Lines        23317    23317           
  Branches      3758     3758           
========================================
  Hits         10466    10466           
  Misses       12851    12851

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 60e11b9...a76e216. Read the comment docs.

@seanlip
Copy link
Member

seanlip commented Feb 11, 2018

One clarification on the original issue, btw: I think we do want the code editor width to depend on the size of the viewport, in exactly the same way as it already does. It just shouldn't depend on the content of the lines within it.

tjiang11
tjiang11 reviewed Feb 12, 2018
View reviewed changes
Copy link
Contributor

@tjiang11 tjiang11 left a comment

Choose a reason for hiding this comment

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

Testing the UI, this looks good! Thanks @bansalnitish ! Though I was wondering if you could just briefly explain how this works (I'm no CSS guru)... it seems as though this would cause the width to be fixed at 650px, but that doesn't seem to be the case.

@bansalnitish
Copy link
Contributor Author

Hey @tjiang11, from what I experienced with the UI, the max-width is attained only when typing is done and otherwise the width stays to be at the minimum. So I fixed the width to 650px and that works fine. Do you want the width to be fixed at 650px or change it to some other fixed value?

tjiang11
tjiang11 approved these changes Feb 12, 2018
View reviewed changes
Copy link
Contributor

@tjiang11 tjiang11 left a comment

Choose a reason for hiding this comment

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

Okay, this seems good. Thanks @bansalnitish !

@tjiang11 tjiang11 merged commit 71c38e6 into oppia:develop Feb 12, 2018
code247 pushed a commit to code247/oppia that referenced this pull request Feb 24, 2018
@bansalnitish @code247
Fixed the position of code editor (oppia#4674)
2b17f18
code247 pushed a commit to code247/oppia that referenced this pull request Feb 24, 2018
@bansalnitish @code247
Fixed the position of code editor (oppia#4674)
976bf5b
@bansalnitish bansalnitish deleted the editor-fix branch March 6, 2018 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tjiang11 tjiang11 tjiang11 approved these changes

Assignees

@tjiang11 tjiang11

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Code Editor should not expand
4 participants
@bansalnitish @codecov-io @seanlip @tjiang11