Fix part of #9749: Migrate base-content #12371

ashutoshc8101 · 2021-03-30T10:43:42Z

Overview

This PR fixes Toast Messages Should Work in Angular components #12386 and fixes part of Migrate directives/controllers to Angular components #9749.
This PR does the following: Migrates base-content directive.

PR is open for some dependencies (top navigation bar, footer) of base-content.
So, I used upgradeComponent for that dependencies. If those PRs get merged before this, I will update this PR to use the migrated components.

Essential Checklist

The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
The linter/Karma presubmit checks have passed locally on your machine.
"Allow edits from maintainers" is checked. (See here for instructions on how to enable it.)
- This lets reviewers restart your CircleCI tests for you.
The PR is made from a branch that's not called "develop".

Proof that changes are correct

base-content:

base-content-2021-03-31_21.54.49.mp4

PR Pointers

Make sure to follow the instructions for making a code change.
Oppiabot will notify you when you don't add a PR_CHANGELOG label. If you are unable to do so, please @-mention a code owner (who will be in the Reviewers list), or ask on Gitter.
For what code owners will expect, see the Code Owner's wiki page.
Make sure your PR follows conventions in the style guide, otherwise this will lead to review delays.
Never force push. If you do, your PR will be closed.
Oppiabot can assign anyone for review/help if you leave a comment like the following: "{{Question/comment}} @{{reviewer_username}} PTAL"
Some of the e2e tests are flaky, and can fail for reasons unrelated to your PR. We are working on fixing this, but in the meantime, if you need to restart the tests, please check the "If your build fails" wiki page.

oppiabot · 2021-03-30T10:45:40Z

Hi @ashutoshc8101, the body of this PR is missing the required description, please update the body with a description of what this PR does.
Also, the karma and linter checklist has not been checked, please make sure to run the frontend tests and lint tests before pushing. Thanks!

oppiabot · 2021-03-30T10:45:43Z

Hi, @ashutoshc8101, this pull request does not have a "CHANGELOG: ..." label as mentioned in the PR checkbox list. Assigning @ ashutoshc8101 to add the required label. PRs without this label will not be merged. If you are unsure of which label to add, please ask the reviewers for guidance. Thanks!

oppiabot · 2021-03-30T10:45:46Z

Hi @ashutoshc8101, when creating WIP/Draft PRs, ensure that your commit messages are prefixed with [ci skip] or [skip ci] to prevent CI checks from running. You can learn more about it here.

oppiabot · 2021-03-30T16:45:55Z

Hi @ashutoshc8101. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks!

ashutoshc8101 · 2021-03-31T14:29:52Z

core/templates/base-components/base-content.component.ts

+    this.getHeaderText = () => {
+      return this.pageTitleService.getPageTitleForMobileView();
+    };
+
+    this.getSubheaderText = () => {
+      return this.pageTitleService.getPageSubtitleForMobileView();
+    };
+  }


Why declare functions inside ngOnInit() ?
Because pageTitleService is undefined while declaring the functions in the conventional way.

Put this in a comment above the two functions.

ashutoshc8101 · 2021-03-31T14:34:39Z

...lates/components/common-layout-directives/navigation-bars/side-navigation-bar.component.html

+  .oppia-sidebar-menu-open {
+  box-shadow: 1px 0 3px rgba(0,0,0,0.12), 1px 0 2px rgba(0,0,0,0.24);
+  height: 110vh;
+  overflow-y: scroll;
+  -ms-transform: translate(0, 0);
+  -webkit-transform: translate3d(0, 0, 0);
+  transform: translate3d(0, 0, 0);
+  visibility: visible;
+  }
+  .oppia-sidebar-menu-open::after {
+    height: 0;
+    opacity: 0;
+    -webkit-transition: opacity 0.5s, width 0.1s 0.5s, height 0.1s 0.5s;
+    transition: opacity 0.5s, width 0.1s 0.5s, height 0.1s 0.5s;
+    width: 0;
+  }
+</style>


Why move this css from oppia.css to component?
Because component's css was taking priority over the global css due to view encapsulation (unlike ajs).

this was causing issues showing sidebar

ashutoshc8101 · 2021-03-31T14:37:25Z

...emplates/components/common-layout-directives/navigation-bars/top-navigation-bar.directive.ts

-            SidebarStatusService.toggleSidebar();
+            // SidebarStatusService.toggleSidebar();
+            // $scope.update = !$scope.update;
+            $scope.toggle();


why this is changed?
Due to change detection issues with upgradeComponent.
When toggleSidebar is called directly from there, change detection only occurs in ajs not in angular.
So, I changed this in such a way that both ajs and angular change detection work.

ashutoshc8101 · 2021-03-31T14:39:55Z

core/templates/pages/exploration-editor-page/exploration-editor-page.mainpage.html

@@ -42,7 +42,7 @@
                  <nav class="navbar navbar-expand navbar-light oppia-navbar oppia-prevent-selection" role="navigation"
                       headroom tolerance="0" offset="0">
                    <div class="navbar-container">
-                      <top-navigation-bar></top-navigation-bar>
+                      <top-navigation-wrapper></top-navigation-wrapper>


to fix the side effects of the changes which were made to fix change detection.

oppiabot · 2021-06-06T05:28:07Z

Unassigning @brianrodri since they have already approved the PR.

oppiabot · 2021-06-06T07:44:44Z

Unassigning @nithusha21 since they have already approved the PR.

ashutoshc8101 · 2021-06-06T11:21:25Z

@sagangwee PTAL

sagangwee

@ashutoshc8101 LGTM for codeowners.

oppiabot · 2021-06-07T15:53:44Z

Unassigning @sagangwee since they have already approved the PR.

This reverts commit 121ee3c.

revert;

889a54b

oppiabot bot assigned ashutoshc8101 Mar 30, 2021

oppiabot bot added the PR: don't merge - HAS MERGE CONFLICTS label Mar 30, 2021

base component migrated

5c0d808

ashutoshc8101 force-pushed the base-components branch from ac90c78 to 5c0d808 Compare March 31, 2021 10:13

ashutoshc8101 added 2 commits March 31, 2021 19:38

done

48ab2fc

Merge remote-tracking branch 'upstream/develop' into base-components

c579ce2

ashutoshc8101 commented Mar 31, 2021

View reviewed changes

oppiabot bot removed the PR: don't merge - HAS MERGE CONFLICTS label Mar 31, 2021

ashutoshc8101 commented Mar 31, 2021

View reviewed changes

ashutoshc8101 added 8 commits March 31, 2021 20:15

nits

ed219fe

nits

0729073

nits

d8eb6ba

nits

28aec1b

nits

5b9ed2e

nits

ba59100

html-lints

bc5cb11

ts lints

b249345

ashutoshc8101 changed the title ~~Base components~~ Fix part of #9749: Migrate base-content, alerts-and-warnings, side-navigation-bar, alert-message and loading-message directives Mar 31, 2021

ashutoshc8101 marked this pull request as ready for review March 31, 2021 16:22

ashutoshc8101 requested review from arpit1912, BenHenning, DubeySandeep and Hudda as code owners March 31, 2021 16:22

oppiabot bot assigned sagangwee Jun 5, 2021

brianrodri self-requested a review June 6, 2021 05:24

brianrodri approved these changes Jun 6, 2021

View reviewed changes

oppiabot bot unassigned brianrodri Jun 6, 2021

srijanreddy98 requested a review from nithusha21 June 6, 2021 07:41

nithusha21 approved these changes Jun 6, 2021

View reviewed changes

oppiabot bot unassigned nithusha21 Jun 6, 2021

ashutoshc8101 added 3 commits June 7, 2021 14:53

Merge remote-tracking branch 'upstream/develop' into base-components

d411011

nit

5612262

nit

719c408

sagangwee removed their assignment Jun 7, 2021

ashutoshc8101 requested a review from sagangwee June 7, 2021 15:48

ashutoshc8101 assigned sagangwee Jun 7, 2021

sagangwee approved these changes Jun 7, 2021

View reviewed changes

oppiabot bot unassigned sagangwee Jun 7, 2021

ashutoshc8101 added 4 commits June 8, 2021 00:54

Merge remote-tracking branch 'upstream/develop' into base-components

0377f5b

Merge remote-tracking branch 'upstream/develop' into base-components

64da1f1

final

f705a9b

nit

7694ce4

ashutoshc8101 enabled auto-merge (squash) June 8, 2021 10:51

ashutoshc8101 disabled auto-merge June 8, 2021 10:51

ashutoshc8101 enabled auto-merge (squash) June 8, 2021 10:51

ashutoshc8101 disabled auto-merge June 8, 2021 14:44

ashutoshc8101 merged commit 121ee3c into oppia:develop Jun 8, 2021

ashutoshc8101 added a commit that referenced this pull request Jun 9, 2021

Revert "Fix part of #9749: Migrate base-content (#12371)"

45b79fb

This reverts commit 121ee3c.

ashutoshc8101 mentioned this pull request Jun 18, 2021

Fix #12855: Remove unused angularjs libraries: angularjs headroom, toastr and ngAudio #13082

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix part of #9749: Migrate base-content #12371

Fix part of #9749: Migrate base-content #12371

ashutoshc8101 commented Mar 30, 2021 •

edited

Loading

oppiabot bot commented Mar 30, 2021

oppiabot bot commented Mar 30, 2021

oppiabot bot commented Mar 30, 2021

oppiabot bot commented Mar 30, 2021

ashutoshc8101 Mar 31, 2021

srijanreddy98 Apr 5, 2021

ashutoshc8101 Apr 5, 2021

ashutoshc8101 Mar 31, 2021

ashutoshc8101 Apr 1, 2021

ashutoshc8101 Mar 31, 2021

ashutoshc8101 Mar 31, 2021 •

edited

Loading

oppiabot bot commented Jun 6, 2021

oppiabot bot commented Jun 6, 2021

ashutoshc8101 commented Jun 6, 2021

sagangwee left a comment

oppiabot bot commented Jun 7, 2021

Fix part of #9749: Migrate base-content #12371

Fix part of #9749: Migrate base-content #12371

Conversation

ashutoshc8101 commented Mar 30, 2021 • edited Loading

Overview

Essential Checklist

Proof that changes are correct

PR Pointers

oppiabot bot commented Mar 30, 2021

oppiabot bot commented Mar 30, 2021

oppiabot bot commented Mar 30, 2021

oppiabot bot commented Mar 30, 2021

ashutoshc8101 Mar 31, 2021

Choose a reason for hiding this comment

srijanreddy98 Apr 5, 2021

Choose a reason for hiding this comment

ashutoshc8101 Apr 5, 2021

Choose a reason for hiding this comment

ashutoshc8101 Mar 31, 2021

Choose a reason for hiding this comment

ashutoshc8101 Apr 1, 2021

Choose a reason for hiding this comment

ashutoshc8101 Mar 31, 2021

Choose a reason for hiding this comment

ashutoshc8101 Mar 31, 2021 • edited Loading

Choose a reason for hiding this comment

oppiabot bot commented Jun 6, 2021

oppiabot bot commented Jun 6, 2021

ashutoshc8101 commented Jun 6, 2021

sagangwee left a comment

Choose a reason for hiding this comment

oppiabot bot commented Jun 7, 2021

ashutoshc8101 commented Mar 30, 2021 •

edited

Loading

ashutoshc8101 Mar 31, 2021 •

edited

Loading