[Bugfix:Submission] Fix vue-id collision#12010
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12010 +/- ##
=========================================
Coverage 21.70% 21.71%
Complexity 9582 9582
=========================================
Files 268 268
Lines 36560 36563 +3
Branches 475 475
=========================================
+ Hits 7936 7938 +2
- Misses 28153 28154 +1
Partials 471 471
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
williamjallen
left a comment
There was a problem hiding this comment.
When we originally set this up, we deliberately didn't want users to set an ID because doing so could lead to conflicts with common names. If our current random approach isn't sufficient, why can't we just use a larger random space such that it's essentially impossible to have a conflict?
|
@williamjallen After a bit of testing, I dont think the correct reason is that theyre colliding. Instead, there are exact duplicates , which does not make much sense to me. It looks like the first 8 vue element's ID are mirrored, messing up the UI in the proceess. Do you have any idea? Moving to uniqid works for vue that is generated PHP side, but doesnt work when using twigjs,which we will still have to use random(). Do you think thats the best option? |
Why is this Change Important & Necessary?
@lavalleeale and I knew that this day would come, where we would have so many vue elements on the screen that the id's conflict with each other and make a very weird site. Therefore, we need to stop using random ids and instead be asserting that it is per script. There are many mistakes in teh gradeable itself, but the part I can show demonstrates it perfectly.
Write your java code is a markdown box that is a vue element. On this page, it seems like it replaces the too many queries banner, the sidebar, and some other things that I did not want to pinpoint. I can confirm this collision as opening it up and checking for the specific ID, there are going to multiple,
What is the New Behavior?
Instead of generating a random ids, we will put the script tag in the div. This also works for removing the script tag as the vue element renders on top on it.
What steps should a reviewer take to reproduce or test the bug or new feature?
I don't know how you can test this reliable, maybe have a large markdown/notebook gradeable.
Automated Testing & Documentation
Other information