fix: update vendor files for SlickGrid to resolve table styling issues

[bra-i-am]

Note

Applying previously working changes from #31643

Description

LMS is using the latest version of the library from http://github.com/mleibman/slickgrid, but it has been inactive for quite some time. The present PR updates the vendor files from https://github.com/6pac/SlickGrid which is the most active fork at the moment.

The error happened in some places because of an override of some css variables

Before

After

Supporting information

#36821

Testing instructions

• Sign in to LMS as a Staff user
• Choose a course and click on View Course
• Go to the Instructor Dashboard by clicking on the Instructor tab
• Switch to the Data Download tab
• Click on the List enrolled students' profile information button

[openedx-webhooks]

Thanks for the pull request, @bra-i-am!

This repository is currently maintained by @openedx/wg-maintenance-openedx-platform.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

• If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
  • This process (including the steps you'll need to take) is documented here.
• If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

• Dependencies

  This PR must be merged before / after / at the same time as ...

• Blockers

  This PR is waiting for OEP-1234 to be accepted.

• Timeline information

  This PR must be merged by XX date because ...

• Partner information

  This is for a course on edx.org.

• Supporting documentation
• Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

---

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

• Overview of Review Process for Community Contributions
• Pull Request Status Guide
• Making changes to your pull request

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

• The size and impact of the changes that it introduces
• The need for product review
• Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

[bra-i-am]

@mariajgrimaldi, here is the merge request I told you I was re-creating

I'll be waiting for your comments, thanks!

[mariajgrimaldi]

Thank you so much for this contribution! Can we test other places where this slickgrid is used to make sure it's still working as expected? Thanks!

[mariajgrimaldi]

Friendly reminder to my question above, @bra-i-am. Thanks!

[bra-i-am]

@mariajgrimaldi, I have upgraded the vendor files to the latest version and made additional modifications to improve the rendering of the tables. I found the issue was related to a styling override that happened only when the course had the tab Open Responses. Although I applied various changes to improve the table display, I didn't find the main source of the issue. The solution to the bug was more like a side effect of upgrading the library. Should I leave it as is, or may I try to find a solution without upgrading the vendor files?

I tested the fix on different tables (the ones I managed to enable) and this is how they are displayed:

Data Download

Extensions

I'm looking forward to your feedback! Thanks ✨

[mariajgrimaldi]

I'm not familiar with the update process of these vendor files but I'd suggest to mention edx-platform maintenance team to validate this is something we want - FYI @openedx/wg-maintenance-edx-platform

Additionally, can we update the PR name and commits to reference the vendor file changes for slickgrid instead?

[kdmccormick]

@bra-i-am , thanks for the PR, and nice fix!

To answer @mariajgrimaldi 's question, there is no established process for updating vendored-in JS and CSS files. Basically, they are neglected. We're looking forward to finishing the MFE conversion so that we can delete these vendored-in files as soon as possible.

That said, this would be a good bugfix to have in Teak.2 and master. I am just nervous about the potential side-effects of applying nine years of SlickGrid upgrades to the platform. 😅 I could two ways forward that'd make me comfortable approving and merging:

1. Find all components in edx-platform which reference SlickGrid, and manually test that they still work on this branch with this latest SlickGrid version.
2. Upgrade to an older version of SlickGrid which still fixes the bug but is not as drastic as 9 years of upgrades.

In either case, I've requested that a sandbox environment be created for this PR in case that'd help you test. You should see a comment with a link to the sandbox soon; the credentials are openedx/openedx.

[open-craft-grove]

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

[open-craft-grove]

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

[open-craft-grove]

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

[open-craft-grove]

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

[mariajgrimaldi]

We have a successfull deployment! @bra-i-am: could you help us testing as Kyle suggested above? I'd say we start documenting how all tables still work with this change.

[open-craft-grove]

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

[mphilbrick211]

Hi @bra-i-am @mariajgrimaldi! Is this pull request still in progress?

[open-craft-grove]

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

[open-craft-grove]

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

[bra-i-am]

@mphilbrick211 @kdmccormick, I'm really sorry for the loong delay. I've been busy with a lot of other things and haven't had time to continue with this. I'm increasing the priority of this in my ToDo to finish it in the following days; it is pending to test that the tables that use SlickGrid are still functional, and I'm missing a few places

@kdmccormick, the sandbox is broken, could you please help me with it? After this, I'll finish the tests asap and leave you the results to proceed with your review

[kdmccormick]

No problem @bra-i-am . The sandbox was probably very old, so I've removed the create-sandbox label so that it got destroyed, and now added it back so that it gets recreated. You should see a deployment within an hour or so. If you see deployment errors that don't make sense, shoot a message to #grove-pr-watcher in Slack.

[bra-i-am]

@kdmccormick, thanks for your help! 🫶

I see the sandbox was not recreated; following your guidance, I will look for help in Slack

[open-craft-grove]

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

[open-craft-grove]

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

[mphilbrick211]

@kdmccormick, thanks for your help! 🫶

I see the sandbox was not recreated; following your guidance, I will look for help in Slack

@bra-i-am is this stil in progress?

[open-craft-grove]

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

[open-craft-grove]

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

[kdmccormick]

@bra-i-am I've removed the create-sandbox label to free up resources, but if you resume working on this PR feel free to put it back.

[mphilbrick211]

@bra-i-am I've removed the create-sandbox label to free up resources, but if you resume working on this PR feel free to put it back.

@bra-i-am is this still in progress?

[mphilbrick211]

@bra-i-am I've removed the create-sandbox label to free up resources, but if you resume working on this PR feel free to put it back.

@bra-i-am is this still in progress?

Friendly ping on this, @bra-i-am!

[mphilbrick211]

Hi @bra-i-am - is this still in progress, or can we close for now?