refactor training pages to allow rendering on client-side instead of … #5523
Conversation
|
Hi @ragesoss . This PR is ready for review. |
|
Thanks! This is a deeper React conversion than what I was suggesting — it converts the entire Training and Module views, rather than just implementing the nav in React and leaving the rest as server-rendered. This is potentially good, and it looks like you've basically got it working! One thing I notice as I try it out is that the 'No Libraries' message flashes briefly whenever you load the training index. The first render of any of these pages should avoid flashing any temporary messages like that. |
| if (!response.ok) { | ||
| const data = await response.text(); | ||
| response.responseText = data; | ||
| throw response; | ||
| } | ||
| const data = await response.json(); |
There was a problem hiding this comment.
this can be extracted to a function — its used in a few places even before this PR
There was a problem hiding this comment.
Hi @TheTrio , sure lemme do that.
| <ul className="training-libraries no-bullets no-margin action-card-text"> | ||
| {slides.map((slide, index) => ( | ||
| <li key={index}> | ||
| <a href={`https://dashboard.wikiedu.org/training/${slide.path}`} target="_blank"> |
There was a problem hiding this comment.
don't think this is correct? The root url will not always be https://dashboard.wikiedu.org/
There was a problem hiding this comment.
Hi, while I didn't make any changes on this and not very sure, I think it will always be that url because the slides controller that has the data is using the TrainingSlide model.
Hi @ragesoss . Sure, lemme check this out. And thank you for the feedback. |
| <div className="training-module-source"> | ||
| <a href={`https://meta.wikimedia.org/wiki/${this.props.training.module.wiki_page}`} target="_blank">{I18n.t('training.view_module_source')}</a> | ||
| <br /> | ||
| <a href={`/reload_trainings?module=${this.props.training.module.slug}`}>{I18n.t('training.reload_from_source')}</a> |
There was a problem hiding this comment.
I think these messages are still relevant, they just shouldn't be shown until after the data has been loaded. If any empty response has already been received from the server, at that point we should show relevant 'missing' messages.
|
Hi @ragesoss, so I was working on getting the module progress and I think this is ready now. However, I have 3 specs failing, in relation to the flashing messages. Initially the messages were only showing when @libraries.empty? is truthy. Currently though, that also seems to be truthy while the data is loading. Can you kindly assist on how I can counter that? I've moved the messages rendering to the controller as before but the specs are still failing. |
|
Hmm... I'm not sure. On the server, @libraries ought to be set by the time it renders, as Rails will wait until the data is loaded in that controller code before moving on. However, I think it would be ideal to move the message to React, and just not show it until after the data has been fetched and received... then only show it at that point if an empty array was received for Libraries. |
|
Hi @ragesoss . So after updating this as advised, there are 2 specs that are failing from the training _tool_spec. However when I try to clear the libraries list using my rails console I get the no libraries message so I'm not entirely sure why it's still failing. |
|
Hmm... I'm not sure what is going wrong. I suggest running the failing test locally with headless mode turned off (by removing the |
sharon-odhiambo
force-pushed
the
refactor-training-pages
branch
from
b9838c0 to
512b3d7
Compare
|
@sharon-odhiambo what's the status of this? it looks like some of the training-specific tests are still failing; were they passing for you locally? anything you are stuck with that i can try to get you unstuck with? |
|
Hi @ragesoss , the tests weren't passing locally when I run them but when I check the page when my database is empty the messages appear for both wiki edu and non-wiki edu mode. I'm not sure how to go about it from here. |
|
Sorry, I didn't get a chance to dive into this to provide advice to get you unstuck. I'm going to close this now, as it overlaps with some work we're doing with the trainings currently, so it will be best to start fresh if/when someone returns to it. |
What this PR does
Whenever a user is logged in wiki-education mode, they are able to access the get help button. The button is however not available when the user launches the training modules from the course page. This PR solves that problem:
Refactoring the training libraries page from the server side rendering by refactoring the index.html.erb to a React component
Refactoring a single library page from the server side to the client side
This PR addresses issue #5486
Screenshots
After:
This video shows the affected pages functionalities and visual appearance.
Screencast from 10-24-2023 02:09:35 AM.webm