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

Fix #3093: Popover of Userhelp button remains constant when the user creates the exploration for the first time. #3267

Merged
merged 8 commits into from
Apr 9, 2017
Merged

Fix #3093: Popover of Userhelp button remains constant when the user creates the exploration for the first time. #3267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4b34d04
Fixed Popover of Userhelp button
shaz13 Apr 1, 2017
97be15f
5 second popup rule active
shaz13 Apr 5, 2017
e89c81a
isLargeScreen condition checked at the function
shaz13 Apr 6, 2017
e803ca1
Indent fixes
shaz13 Apr 6, 2017
95c67df
Linter fix
shaz13 Apr 6, 2017
f475eb8
Scoped CSS and isLargeScreen scope replacement
shaz13 Apr 8, 2017
f8a4d88
Class specific with div
shaz13 Apr 9, 2017
9f86a93
oppia-help-dropdown classe sorted
shaz13 Apr 9, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions core/templates/dev/head/pages/exploration_editor/EditorNavigationDirective.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,17 @@ oppia.directive('editorNavigation', [function() {
threadDataService, siteAnalyticsService,
explorationContextService, windowDimensionsService) {
$scope.postTutorialHelpPopoverIsShown = false;
$scope.isLargeScreen = (windowDimensionsService.getWidth() >= 1024);

$scope.$on('openPostTutorialHelpPopover', function() {
$scope.postTutorialHelpPopoverIsShown = true;
$timeout(function() {
if ($scope.isLargeScreen) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Move $scope.isLargeScreen above this function, this is so that we can get that defined before this gets a chance to trigger.

Copy link
Member

Choose a reason for hiding this comment

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

Actually -- I think everything gets initialized before anything runs, so the existing code is fine. Does it not trigger on your end?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, on second thought (and trial), it is fine, though it might be better if we shift those variable instantiation higher up for readability's sake.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kevinlee12 openPostTutorialHelpPopover triggers when the user comes to create explorations for the first time.
If adopted this code:

   if ($scope.isLargeScreen) {
            $scope.$on('openPostTutorialHelpPopover', function() {
            $scope.postTutorialHelpPopoverIsShown = true;
            $timeout(function() {
              $scope.postTutorialHelpPopoverIsShown = false;
            }, 5000);
            });
          } else {
            $scope.postTutorialHelpPopoverIsShown = false;
          }

The helptool tip doesnt appear at all. Because, scripts of isLargeScreen execute first and misses openPostTutorialHelpPopover before it could initialize.

Copy link
Contributor

Choose a reason for hiding this comment

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

nope, that's not what I meant. I was speaking of moving line 103 above this function.

Copy link
Contributor

Choose a reason for hiding this comment

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

this line: $scope.isLargeScreen = (windowDimensionsService.getWidth() >= 1024);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kevinlee12 Sure. Will push another commit on it

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

$scope.postTutorialHelpPopoverIsShown = true;
$timeout(function() {
$scope.postTutorialHelpPopoverIsShown = false;
}, 5000);
} else {
$scope.postTutorialHelpPopoverIsShown = false;
}, 5000);
}
});

$scope.showUserHelpModal = function() {
Expand Down Expand Up @@ -96,7 +101,6 @@ oppia.directive('editorNavigation', [function() {
$scope.selectHistoryTab = routerService.navigateToHistoryTab;
$scope.selectFeedbackTab = routerService.navigateToFeedbackTab;
$scope.getOpenThreadsCount = threadDataService.getOpenThreadsCount;
$scope.isLargeScreen = (windowDimensionsService.getWidth() >= 1024);

windowDimensionsService.registerOnResizeHook(function() {
$scope.isLargeScreen = (windowDimensionsService.getWidth() >= 1024);
Expand Down
12 changes: 11 additions & 1 deletion core/templates/dev/head/pages/exploration_editor/editor_navigation_directive.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,24 @@
</li>

{% if username %}
<li ng-if="isLargeScreen" class="dropdown" popover-is-open="postTutorialHelpPopoverIsShown"
<div class="nav navbar-nav oppia-navbar-nav oppia-help-dropdown pull-right ng-cloak">
<li class="dropdown" popover-is-open="postTutorialHelpPopoverIsShown"
popover-placement="bottom" popover-trigger="none"
popover="To get help in the future, click here!">
<a href="#" tooltip="Help" tooltip-placement="bottom"
class="oppia-editor-navbar-tab-anchor" ng-click="showUserHelpModal()">
<i class="material-icons">&#xE887;</i>
</a>
</li>
</div>
{% endif %}
</ul>
</script>
<style type="text/css">
.oppia-help-dropdown .popover {
font-size: 13px;
max-width: 200px;
text-align: center;
z-index: -1;
}
</style>