-
Notifications
You must be signed in to change notification settings - Fork 226
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
ENH Rewrite front-end JS in vanilla JS #1197
base: 5
Are you sure you want to change the base?
ENH Rewrite front-end JS in vanilla JS #1197
Conversation
Interesting start, keen to see results of the tests. It's also quite hard to read the JS file diff since everything changed. |
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 assume based on the failed tests that this isn't ready yet - should this be a draft?
@GuySartorelli for some modules tests are failing on composer install, it can't get some core dependencies. Similar for e.g. contentreview module. It's not a functional issue, it doesn't even get to testing that. |
Ahh, I didn't even think to click through to see if the failures were something like that 😅 |
@fonsekaean Just to be clear, as I see you're occasionally making new commits to this - is this still a work in progress, or is it ready for review? |
@GuySartorelli its all done, I was testing it on a project and found a specific issue that I sorted. |
did another commit and that should be the last one @GuySartorelli |
And another one.. I reckon we probably need a wider testing/more experience with the change before we accept the PR. |
The pagination and progress title failing tests seem relevant. |
I will check that one out. Also did some slight updates today as well |
This removed the requirement of jQuery on front end validations, display rules and step navigations.