Autofocus: Adds autofocus to learner dashboard, contributor dashboard and story editor. #12114

Rijuta-s · 2021-03-06T08:58:18Z

Overview

This PR fixes or fixes part of nothing
This PR does the following: Adds autofocus to learner dashboard,preference page, contributor dashboard and story editor

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:

Screencast.from.17-03-21.11_34_38.AM.IST.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-06T08:58:19Z

Hi, @Rijuta-s, this pull request does not have a "CHANGELOG: ..." label as mentioned in the PR checkbox list. Assigning @ Rijuta-s 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!

vojtechjelinek

LGTM!

oppiabot · 2021-03-07T19:21:28Z

Assigning @nithusha21 for code owner reviews. Thanks!

kevintab95

Thanks @Rijuta-s! Just 2 questions, PTAL!

kevintab95 · 2021-03-08T10:43:36Z

core/templates/pages/story-editor-page/editor-tab/story-editor.directive.ts

-                () => _init()
+                () =>{
+                  _init();
+                  setTimeout(() => {


Why is a timeout required?

To ensure that this code runs only after the script is compiled and the component is visible in the page and is not going any changes due to the loading script..this was the only reason making sense to me, at these place without set time the code was not puling up focus, so.....I'm not completely sure of the reason.

kevintab95 · 2021-03-08T10:43:51Z

core/templates/pages/story-editor-page/editor-tab/story-node-editor.directive.ts

@@ -432,6 +434,9 @@ angular.module('oppia').directive('storyNodeEditor', [
              )
            );
            _init();
+            setTimeout(() => {


Ditto: Why setTimeout?

same as above.

Thanks, please add a comment in the code.

Also, does this work all the time?

It works everytime I have tested at this place.I have added the comment

oppiabot · 2021-03-08T10:47:05Z

Unassigning @kevintab95 since the review is done.

nithusha21

LGTM for codeowner files!

oppiabot · 2021-03-08T15:00:03Z

Unassigning @nithusha21 since they have already approved the PR.

oppiabot · 2021-03-09T05:11:10Z

Hi @Rijuta-s, the build of this PR is stale and this could result in tests failing in develop. Please update this pull request with the latest changes from develop. Thanks!

srijanreddy98

LGTM!

U8NWXD · 2021-03-18T23:49:27Z

core/tests/protractor_desktop/preferences.js

@@ -144,11 +144,13 @@ describe('Preferences', function() {
      await users.createUser('lorem@preferences.com', 'loremPreferences');
      await users.login('lorem@preferences.com');
      await preferencesPage.get();
+      await waitFor.pageToFullyLoad();


Could you add this to the end of the preferencesPage.get() function instead? I think that will make the code cleaner. Also, note that waitFor.pageToFullyLoad(); really waits for a loading indicator to disappear, so please make sure that such an indicator is actually displayed when loading the preferences page

Actually, I don't think this does anything. We already have a waitFor.pageToFullyLoad() at the end of the preferencesPage.get() function:

oppia/core/tests/protractor_utils/PreferencesPage.js

Lines 69 to 72 in 010af88

this.get = async function() {

await browser.get(USER_PREFERENCES_URL);

await waitFor.pageToFullyLoad();

};

I think the failures you're seeing must be due to something else

Done. I have removed autofocus from preference page currently. It will be done later in another PR.Thank you

oppiabot · 2021-03-21T16:17:16Z

Hi @Rijuta-s, the build of this PR is stale and this could result in tests failing in develop. Please update this pull request with the latest changes from develop. Thanks!

oppiabot · 2021-03-23T04:55:18Z

Hi @Rijuta-s, there is a new change in develop which needs to be in your PR. Please update your branch with the latest changes in develop. For instructions, refer to this link. Thanks!

…ashboards

Rijuta-s · 2021-03-25T14:45:40Z

@U8NWXD PTAL! thank you

oppiabot · 2021-03-27T00:04:51Z

Hi @Rijuta-s, there is a new change in develop which needs to be in your PR. Please update your branch with the latest changes in develop. For instructions, refer to this link. Thanks!

…ashboards

Rijuta-s added 2 commits March 6, 2021 14:00

adding autofocus to contributor,

638a5c4

minor fixes

ccaab79

Rijuta-s requested review from jameesjohn, kevintab95, nithusha21 and sagangwee as code owners March 6, 2021 08:58

Rijuta-s requested review from srijanreddy98 and vojtechjelinek as code owners March 6, 2021 08:58

oppiabot bot assigned Rijuta-s Mar 6, 2021

Rijuta-s added the PR CHANGELOG: Learner/Creator Pages -- @aks681 label Mar 6, 2021

Rijuta-s assigned sagangwee, vojtechjelinek, kevintab95, srijanreddy98 and jameesjohn Mar 7, 2021

vojtechjelinek approved these changes Mar 7, 2021

View reviewed changes

vojtechjelinek removed their assignment Mar 7, 2021

oppiabot bot assigned nithusha21 Mar 7, 2021

kevintab95 requested changes Mar 8, 2021

View reviewed changes

oppiabot bot unassigned kevintab95 Mar 8, 2021

nithusha21 approved these changes Mar 8, 2021

View reviewed changes

oppiabot bot unassigned nithusha21 Mar 8, 2021

oppiabot bot added the PR: don't merge - STALE BUILD The build on this PR is stale and should be restarted. label Mar 9, 2021

Rijuta-s requested a review from kevintab95 March 9, 2021 14:48

srijanreddy98 approved these changes Mar 10, 2021

View reviewed changes

Rijuta-s assigned U8NWXD and unassigned aks681 and Rijuta-s Mar 18, 2021

U8NWXD requested changes Mar 18, 2021

View reviewed changes

U8NWXD assigned Rijuta-s and unassigned U8NWXD Mar 18, 2021

U8NWXD removed the PR: LGTM label Mar 18, 2021

oppiabot bot added the PR: don't merge - STALE BUILD The build on this PR is stale and should be restarted. label Mar 21, 2021

Rijuta-s added 3 commits March 23, 2021 18:18

Merge remote-tracking branch 'upstream/develop' into dashboards

bca472f

applying suggestions

b9b267b

Merge branch 'dashboards' of https://github.com/Rijuta-s/oppia into d…

36c497d

…ashboards

oppiabot bot removed the PR: don't merge - STALE BUILD The build on this PR is stale and should be restarted. label Mar 23, 2021

Rijuta-s added 2 commits March 25, 2021 00:47

Update preferences-page.component.ts

f9b79af

Update preferences-page.component.ts

f29a8c1

Rijuta-s changed the title ~~Autofocus: Adds autofocus to preference, contributor dashboard and story editor.~~ Autofocus: Adds autofocus to learner dashboard, contributor dashboard and story editor. Mar 24, 2021

Rijuta-s requested a review from U8NWXD March 24, 2021 19:21

Update preferences-page.component.html

f80cd97

Rijuta-s assigned U8NWXD and unassigned Rijuta-s Mar 24, 2021

oppiabot bot assigned Rijuta-s Mar 27, 2021

Rijuta-s added 2 commits March 28, 2021 09:53

Merge remote-tracking branch 'upstream/develop' into dashboards

1128112

Merge branch 'dashboards' of https://github.com/Rijuta-s/oppia into d…

f02e3a6

…ashboards

Rijuta-s removed their assignment Mar 29, 2021

U8NWXD approved these changes Mar 29, 2021

View reviewed changes

U8NWXD merged commit ed8c395 into oppia:develop Mar 29, 2021

Rijuta-s deleted the dashboards branch April 15, 2021 06:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Autofocus: Adds autofocus to learner dashboard, contributor dashboard and story editor. #12114

Autofocus: Adds autofocus to learner dashboard, contributor dashboard and story editor. #12114

Rijuta-s commented Mar 6, 2021 •

edited

Loading

oppiabot bot commented Mar 6, 2021

vojtechjelinek left a comment

oppiabot bot commented Mar 7, 2021

kevintab95 left a comment

kevintab95 Mar 8, 2021

Rijuta-s Mar 9, 2021

kevintab95 Mar 8, 2021

Rijuta-s Mar 9, 2021

kevintab95 Mar 16, 2021

Rijuta-s Mar 17, 2021

oppiabot bot commented Mar 8, 2021

nithusha21 left a comment

oppiabot bot commented Mar 8, 2021

oppiabot bot commented Mar 9, 2021

srijanreddy98 left a comment

U8NWXD Mar 18, 2021

U8NWXD Mar 18, 2021 •

edited

Loading

Rijuta-s Mar 24, 2021

oppiabot bot commented Mar 21, 2021

oppiabot bot commented Mar 23, 2021

Rijuta-s commented Mar 25, 2021

oppiabot bot commented Mar 27, 2021

	this.get = async function() {
	await browser.get(USER_PREFERENCES_URL);
	await waitFor.pageToFullyLoad();
	};

Autofocus: Adds autofocus to learner dashboard, contributor dashboard and story editor. #12114

Autofocus: Adds autofocus to learner dashboard, contributor dashboard and story editor. #12114

Conversation

Rijuta-s commented Mar 6, 2021 • edited Loading

Overview

Essential Checklist

Proof that changes are correct:

PR Pointers

oppiabot bot commented Mar 6, 2021

vojtechjelinek left a comment

Choose a reason for hiding this comment

oppiabot bot commented Mar 7, 2021

kevintab95 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oppiabot bot commented Mar 8, 2021

nithusha21 left a comment

Choose a reason for hiding this comment

oppiabot bot commented Mar 8, 2021

oppiabot bot commented Mar 9, 2021

srijanreddy98 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

U8NWXD Mar 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oppiabot bot commented Mar 21, 2021

oppiabot bot commented Mar 23, 2021

Rijuta-s commented Mar 25, 2021

oppiabot bot commented Mar 27, 2021

Rijuta-s commented Mar 6, 2021 •

edited

Loading

U8NWXD Mar 18, 2021 •

edited

Loading