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 #4683: Altered positioning of help card to make it appropriate #4696

Merged
merged 3 commits into from
Feb 15, 2018
Merged

Fix #4683: Altered positioning of help card to make it appropriate #4696

merged 3 commits into from
Feb 15, 2018

Conversation

nithusha21
Copy link
Contributor

Fix #4683. I have added a function that returns the top position of the help card. This function will return a minimum value of 0 so it will at least be aligned to the top of the supplemental interaction card (and the page will expand below). If the card is not that big, it is centered about the bottom edge of the image. Does this look ok?

Checklist

  • The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes.
  • The PR explanation includes the words "Fixes #bugnum: ...".
  • 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.

@codecov-io
Copy link

codecov-io commented Feb 14, 2018 •
edited
Loading

Codecov Report

Merging #4696 into develop will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4696      +/-   ##
===========================================
- Coverage    44.84%   44.83%   -0.01%     
===========================================
  Files          384      384              
  Lines        23339    23343       +4     
  Branches      3769     3769              
===========================================
  Hits         10467    10467              
- Misses       12872    12876       +4
Impacted Files Coverage Δ
...es/exploration_player/SupplementalCardDirective.js 2.17% <0%> (-0.21%) ⬇️

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 181e8ec...2649ecd. Read the comment docs.

@seanlip seanlip removed their request for review February 14, 2018 20:11
@tjiang11
Copy link
Contributor

LGTM! Thanks @nithusha21 !

@tjiang11 tjiang11 merged commit bb2d526 into oppia:develop Feb 15, 2018
@nithusha21
Copy link
Contributor Author

Thank you @tjiang11

tjiang11 pushed a commit that referenced this pull request Feb 15, 2018
@nithusha21 @tjiang11
Fix #4683: Altered positioning of help card to make it appropriate (#…
68c5bd0 
…4696)

* fix positioning of help card

* fixed linting

* minor fix
code247 pushed a commit to code247/oppia that referenced this pull request Feb 24, 2018
@nithusha21 @code247
Fix oppia#4683: Altered positioning of help card to make it appropria…
5b6c5e9 
…te (oppia#4696)

* fix positioning of help card

* fixed linting

* minor fix
code247 pushed a commit to code247/oppia that referenced this pull request Feb 24, 2018
@nithusha21 @code247
Fix oppia#4683: Altered positioning of help card to make it appropria…
23752f9 
…te (oppia#4696)

* fix positioning of help card

* fixed linting

* minor fix
ctao5660 pushed a commit to ctao5660/oppia that referenced this pull request Nov 23, 2018
@nithusha21 @tjiang11
Fix oppia#4683: Altered positioning of help card to make it appropria…
6ae5918 
…te (oppia#4696)

* fix positioning of help card

* fixed linting

* minor fix
ashutoshc8101
ashutoshc8101 reviewed Sep 9, 2021
View reviewed changes
core/templates/dev/head/pages/exploration_player/SupplementalCardDirective.js
$scope.getHelpCardBottomPosition = function() {
var helpCard = $('.conversation-skin-help-card');
var container = $('.conversation-skin-supplemental-card-container');
return Math.max(container.height() - helpCard.height() / 2, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

@nithusha21 I am working on migrating this directive but failing to understand this formula.
Can you please answer the following questions?

  1. What is the ideal position of the help card. Where should it be shown to the user?
  2. How this formula achieves the expected positioning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't remember to be frank. If I did actually make this change, it was probably because the help card was getting hidden below the footer, which made the continue button not clickable. To fix this we moved it further up. Ideal position being bottom right, but above footer (make sure to check different screen sizes)

Copy link
Contributor

@ashutoshc8101 ashutoshc8101 Sep 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks, I have got the answer which I was looking for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashutoshc8101 ashutoshc8101 ashutoshc8101 left review comments

@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.
5 participants
@nithusha21 @codecov-io @tjiang11 @ashutoshc8101 @seanlip