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.

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

[BUG]: Console error in chapter editor page. #19816

Open
prafulbbandre opened this issue Feb 25, 2024 · 6 comments · May be fixed by #20219
Open

[BUG]: Console error in chapter editor page. #19816

prafulbbandre opened this issue Feb 25, 2024 · 6 comments · May be fixed by #20219
Assignees
Labels
bug Label to indicate an issue is a regression Impact: Medium Will improve quality-of-life for at least 30% of users. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.

Comments

@prafulbbandre
Copy link
Contributor

Describe the bug

console error is seen in chapter editor page.

URL of the page where the issue is observed.

https://www.oppiatestserver.org/story_editor/wvGQpYllJzVf#/chapter_editor/node_1

Steps To Reproduce

visit the link provided in the url section.

Expected Behavior

no console error should be there.

Screenshots/Videos

ZZZffdsfdfdsfdsfdsfdsfds - Oppia - Google Chrome 25-02-2024 11_39_45

What device are you using?

Desktop

Operating System

Windows

What browsers are you seeing the problem on?

Chrome

Browser version

Version 121.0.6167.161 (Official Build) (64-bit)

Additional context

No response

Tips for developers

Before addressing the bug, please identify which PR caused the issue (you can follow the steps here). If you identify the PR, comment on the issue with a link to it. If not, mention the commit hash of the oldest commit you saw the bug on (and the month and year it was made in).

Also, if this is your first issue, please make sure to follow https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#choosing-a-good-first-issue and https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#setting-things-up before claiming it. Thanks!

@prafulbbandre prafulbbandre added triage needed bug Label to indicate an issue is a regression labels Feb 25, 2024
@seanlip seanlip added Impact: Low -- DO NOT WORK ON THIS YET Postponing for now, since it doesn't affect users much. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. Impact: Medium Will improve quality-of-life for at least 30% of users. and removed triage needed Impact: Low -- DO NOT WORK ON THIS YET Postponing for now, since it doesn't affect users much. labels Feb 25, 2024
@kevintab95
Copy link
Member

This issue occurs on the 3.3.5 branch as well.

@Sepremo
Copy link

Sepremo commented Apr 3, 2024

@seanlip Hello! I believe I have resolved these console errors by changing where the values are set to be more similar to the way they were set when on the subTopic page. While doing so, I observed that the 'header text' and 'subheader text' values were not being displayed anywhere, despite being defined for the Mobile Screen due to the condition !WindowIsNarrow. Therefore, I removed the '!', added the condition to check if the title exists and corrected the display of those values on the frontend. Is this the desired outcome?
Could I be assigned this?

19816.Fixed.mp4

Before:
Before
After:
After

Sepremo added a commit to Sepremo/oppia that referenced this issue Apr 5, 2024
@seanlip
Copy link
Member

seanlip commented Apr 9, 2024

@Sepremo Sorry for the late reply. I think what we would need to know, is what the cause of the original error is. Can you explain it? (Otherwise we cannot be sure that your fix addresses the root cause of the error.)

I'm also finding it hard to understand what exactly you changed and the reasoning behind it. Perhaps you could explain that in more detail?

Thanks!

@Sepremo
Copy link

Sepremo commented Apr 9, 2024

@seanlip

@Sepremo Sorry for the late reply. I think what we would need to know, is what the cause of the original error is. Can you explain it? (Otherwise we cannot be sure that your fix addresses the root cause of the error.)

Certainly! The error occurred when attempting to open the Chapter Editor in the Story Editor. It appears that the headerText (along with subheaderText) property undergoes updates after Angular's change detection cycle has concluded. This behavior stems from the component managing the new Chapter Editor Tab. The issue arises because it alters a bound value (as bound in the top-navigation-bar.component.html) within a new Tab (Chapter Editor), albeit within the same page (Story Editor).

Upon further investigation, I noticed a similar scenario where opening a subTopic Editor tab from the Topic Editor page did not encounter this error. The disparity lies in how the headerText (and subheaderText) values were updated: in the TopicEditorRoutingService instead of within the subTopic Editor component, unlike the approach adopted by the Chapter Editor.

I'm also finding it hard to understand what exactly you changed and the reasoning behind it. Perhaps you could explain that in more detail?

To address the issue, I relocated the updates for the headerText and subheaderText values from the Chapter Editor component (story-node-editor.component.ts) to the equivalent in the Story Editor, specifically within the TopicEditorRoutingService (found in story-editor-navigation.service.ts). This adjustment aimed to ensure consistency and resolve the encountered error.

Upon further examination, I observed that these values failed to display even when I resized the screen to simulate a mobile device. I discovered that the values were not visible when the windowIsNarrow flag was set to true, which appeared contradictory. Consequently, I modified the display behavior to rectify this issue and addressed some display inconsistencies by refining the CSS in the Top Navigation Bar.
Is this the expected behavior?

Thanks!

You're welcome! I'm sorry if I explained anything poorly.

@seanlip
Copy link
Member

seanlip commented Apr 11, 2024

Thanks @Sepremo! This all sounds reasonable, feel free to create a PR. I'll assign you to the issue so that you can do that. Thanks!

@Sepremo
Copy link

Sepremo commented Apr 14, 2024

@seanlip
Hello! I apologize for the delay in creating a PR. After completing a rebase without encountering any conflicts, it appears that my code is failing the MyPy checks (AssertionError: Cannot find component 'TraceDispatch' for 'prompt_toolkit.application.application.Application.TraceDispatch').
When checking each commit from my rebase, the error seems to appear when adding the Voice artist metadata audit and model creation job (#19742) commit. I am currently investigating to identify the root cause of the issue and will create a PR as soon as I can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Label to indicate an issue is a regression Impact: Medium Will improve quality-of-life for at least 30% of users. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.
Projects
Status: In Progress
4 participants