Solved the page title Error #11338
Solved the page title Error #11338
Conversation
Signed-off-by: shivangrawat30 <[email protected]>
github-actions
bot
added
APP: Coach
| this.$router.replace( | ||
| this.$router.getRoute('PLAN_LESSONS_ROOT', { classId: this.classId }), | ||
| this.$router.replace( | ||
| this.$router.getRoute('PLAN_LESSONS_ROOT'), |
There was a problem hiding this comment.
thank you for your contribution, @ShivangRawat30 - we appreciate your work on this!
i believe a different approach to solving this problem will have to be explored. the code removed by this PR on line 119, { classId: this.classId }), was only recently added to our codebase (also, see here for the initial implementation of the change elsewhere in Coach) and it serves a specific and important purpose. this code shares the relevant classId with the PLAN_LESSONS_ROOT page, and the page needs that information to render correctly.
in specific use cases, if the page does not know classId, PLAN_LESSONS_ROOT has no awareness of which list of lessons to show the user. lessons are tied to a specific classId and in cases where a facility has multiple classes or a user is an admin of more than one facility, ensuring we have the correct classId is more complicated than it is for users who belong to a single facility that has one class.
since it would result in broken functionality, we aren't able to incorporate this suggested change, but if you're interested in continuing to work on it, i think it's worth exploring alternate solutions! :)
There was a problem hiding this comment.
I'm keen on persisting with the issue and exploring alternative solutions.
There was a problem hiding this comment.
excellent - please feel free to re-request my review when you have an update!
Build Artifacts
|
Signed-off-by: shivangrawat30 <[email protected]>
|
@thanksameeelian Please review the changes, I think now there won't be any issue with |
|
@ShivangRawat30 thanks for your persistence here! your solution here would ultimately lead to the same effects as the solution you initially put forth - effectively, the we would like to introduce a fix for this issue that addresses the reasons the error is being thrown, rather than suppressing that error. it may help to investigate where the error is being thrown, because we are throwing an error when none should be occurring, as seen in the screen recording for this issue. with our current code, navigation is happening successfully and without any notable delay or unexpected UI effects, but we are seeing errors in the console because we are calling the pageTitle code at a time when it does not need to be called, during the midst of navigation. |
|
I will close this due to long time of inactivity. Thank you, @ShivangRawat30. In case you'd like to return to it, let us know. |
|
I will look into this PR, my exams were going so I couldn't look into it. |
|
@ShivangRawat30 Sure! Let us know and we can re-open, but perhaps it'd be simpler to open a new PR since it seems that a different approach will be needed. It's up to you. You can just post a comment in the issue again and we will assign you. Good luck with your exams :) |
|
@MisRob I'll make a new pull request for it. |
|
Okay, thank you @ShivangRawat30 |
This PR solves the overly aggressive errors from page title logic as state is changing in Coach.
WhatsApp.Video.2023-09-30.at.12.37.00.AM.mp4
References
This PR fixes #11291
Reviewer guidance
on the lesson's Summary page, Options > Delete lesson & confirm choice in the modal
should be automatically navigated back to class's Plan > Lessons page
Testing checklist
PR process
Reviewer checklist
yarnandpip)