-
Notifications
You must be signed in to change notification settings - Fork 631
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
Raised for testing purpose - Contains all code of GSoC 2024 #5946
base: master
Are you sure you want to change the base?
Raised for testing purpose - Contains all code of GSoC 2024 #5946
Conversation
This is an impressive set of functionality. I've done some initial testing, and there are a few UX problems that I think will need to be fixed before this is ready to deploy.
|
I also encountered a JS error when entering 'reorder' mode on an existing module's index page:
|
Fixing this error and also reducing some unnecessary lines of code of 'reordering slides' pr. Then will move on to incorporating the listed changes to make it perfect for deployment. |
sir, where you found this error as, I tried replicating this error in my browser console but found nothing |
@ragesoss sir, kindly have a look |
Hmm... I still hit that same error. I'm doing this in wiki_education mode, with freshly reloaded training modules, and any time I click 'Change Order' on the index page of one of the YAML-backed modules, it throws a JS error and the React-rendered sidebar disappears. |
<div className="draggable-slide-container"> | ||
<div className="program-description__header"> | ||
<h4><strong>{heading}</strong></h4> | ||
{description.split('\n').map((paragraph, i) => paragraph && <p key={i}>{paragraph}</p>)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is where it errors when I enter reorder mode for a yml-based module. description
comes from slide.wiki_page
, so when the wiki_page is null, it will error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a null check there.
596a1a4
to
9125c59
Compare
@ragesoss sir, kindly verify that js error that we have encountered earlier is not happening now ? |
Yep! That fixes the error. Thanks! I will continue testing from here, this week or next. |
What this PR does
This pr is created for testing all training modification features.
Eg: how they work over production server