-
Notifications
You must be signed in to change notification settings - Fork 15
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
[TC_LEARNER_39] Notes tab linking not working #233
Comments
label: olive testing |
Hi @jfavellar90, are you planning to work on fixing this issue or should we tag it as "help wanted" to move it forward? |
Hi @jfavellar90, any news on this? @DeanJayMathew is this issue still happening? |
@jalondonot sorry for the late answer. I can take over this one to check if this is still happening, and if that's the case, try to propose a solution, otherwise, I'd tag this issue as "help wanted" |
@jalondonot last 2 weeks I haven't had the time to push this task. I'll be putting aside some time this week and I'll post an update on this task. |
@jfavellar90, do hoy have any updates on this? |
@jalondonot this is still WIP. It is still happening in Olive and I think the best approach is to move the notes page to a micro-frontend (it is still served via LMS directly). This bug should not be considered a blocker at all for palm cutoff. @arbrandes is it OK if I move forward and work on this one? I need assistance finding the best place to put this page in the micro-frontends. |
@jfavellar90, any work on moving pages to MFEs is most welcome, indeed. :) To avoid coordination issues, I suggest you make this a stand-alone MFE. Like with the Discussions MFE, if enabled, clicking on the Notes tab would redirect to the (theoretical) Notes MFE. |
@jfavellar90: are these two (#233 and #227) issues related? If they are, can we close one? So it's easier to maintain the status up to date |
@jfavellar90, for the record, there's no need to wait on the Modular Domains work if you want to get started on this. However, I would strongly suggest mocking up the feature (figma, maybe?) and getting some product feedback (is it going to have 1-to-1 feature parity with the current notes frontend?) before actually diving into the code. |
@mariajgrimaldi These issues are unrelated so for me, it makes sense to manage them separately. |
With a designer from my company we will make a prototype for the new mfe of notifications, we will be commenting when we make progress and we will ask for comments |
Just to complement the comment from @santiagosuarezedunext , we met internally to explain the technical problem to my teammates and the next step for us will be the creation of a prototype for the stand-alone notes MFE and socialize it to get feedback. I'm not sure if this issue "[TC_LEARNER_39] Notes tab linking not working" maps properly the work we are going to do. @arbrandes should we create a new issue related to this New MFE creation? Not sure if it should land here in the BTR WG or better to move it to the Frontend WG |
Hello, together with our designer @maguilarUXUI we created the following prototype that updates the notes module based on Paragon to do it in an MFE. |
As @santiagosuarezedunext mentions, this proposal is based on the current functionality of notes, and the variations are minimal. We rely on the Paragon Design System for the UI. |
This is great! I think this is a good time to bring product into it. Can you folks make it into one of the upcoming Product WG meetings to present this? This would also be a good conversation for one of the upcoming frontend WG ones, too. |
@arbrandes A little update |
Hello @santiagosuarezedunext! Would you happen to know the status of this issue? We'd appreciate any update. Thanks! |
I'm not sure what the state of this is since Santiago is no longer working on this project, but I think this should be a product proposal instead. So this issue addresses the current issues with notes, maybe linking to the product proposal as a long-term proposal. |
@crathbun428: what do you think about my comment here? |
@mariajgrimaldi - To me this reads as a bug to be fixed since I'd expect when I click the unit name of the note I made, I'd expect to go to that unit (the unit where I made the note), not the last unit I was at. Since the desired behavior is not in question here, but more broken, I wouldn't put this through the product proposal review process. I'd only do that if there was a question about the desired behavior. Is there another reason you were thinking this would need to go through the Product Proposal Review Process? I don't think this is a high-priority bug since not all instances use the Notes app, more of a medium priority bug. |
@crathbun428: Sorry for not being clearer. This issue has been open for a while and covers different things. One part is Santiago's proposal in the Product WG to turn the notes tab into an MFE, which I think started here: #233 (comment). I think we should move that discussion elsewhere - my bad. For now, I'll focus on fixing this bug without considering the MFE transition. |
Ah - got it. We can take a look at where this proposal stands in the product review process next product working group meeting (next Tuesday morning). |
For anyone picking up this issue, here's a quick summary and the definition of done for this issue: Summary The Notes tab links do not redirect to the correct unit. Instead, they take users to the last visited unit. This happens because the Notes page is served by the LMS and does not properly translate legacy links to MFE links. Definition of Done (DoD)
The MFE transition is a separate effort and not part of this fix. |
Links in the notes tab are not redirecting to the right units containing student notes.
Steps:
Enable Notes in a Course Advanced settings
Open Course as a Student
Check Notes tab
Open any Unit in a subsection
Select text to note
Save the note
Advance to the next unit in the sequential
Check the Notes tab
Follow the note link in the Notes tab
Check you're in the last visited unit, not the one containing the note
Status: FAILED
I think this is due to the fact that the notes tab page is outside the MFE (is served by LMS directly) so it is using legacy links that are being translated properly to MFE links
The text was updated successfully, but these errors were encountered: