- Issue created by @balintbrews
- πΊπΈUnited States tedbow Ithaca, NY, USA
We still might not want to do this right now but I think we would not have hit π Unable to add SDCs with slots to the page Active if we had the test.
It seems like "slots" is a very basic functionality for XB and we don't know if any commit will break that should be consider "major" priority to fix it.
could we at least add back the part of the test that drags the "two column" element on? - πΊπΈUnited States tedbow Ithaca, NY, USA
I know this issue was about testing for dragging components into slots but since we skip
components-slots.cy.js
completely doesn't that mean we also don't have e2e coverage for dragging a component with slots on the canvas? Could we do just the part of the test where we add the "Two column" component on to the canvas and then fix the rest later?I think we would not have had the regression in π Unable to add SDCs with slots to the page Active if we at least had that.
If seems like from the issue summary that problem is testing placing items in the slot not placing an item with slots the canvas, correct?
I think we should add test coverage soon for what we can
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
@tedbow++ β bumping priority.
- π¬π§United Kingdom jessebaker
I don't think is particularly valuable to have an end to end test that ACTUALLY drags and drops elements (e.g. mousedown, mousemove, mouseover, mouseup etc) . It is a lot of work, is prone to failure etc. And we use a 3rd party library that I assume has its own tests anyway. Furthermore it's immediately obvious to any human that it is broken as it's such a core piece of the XB functionality so it's unlikely to get missed for any length of time.
Instead, I wonder if we can find a way to programmatically call the same reducer function that occurs on the drop event directly from Cypress. This would ensure the same code runs (updating the JSON, sending the JSON to the server, receiving the updated HTML, updating the preview) without having to actually do all the fiddly stuff of emulating mouse moves and hover events etc.
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Based on a retrospective discussion we just had: +1 to #7. I did not realize in #5 that it was so onerous to write this, but upon re-reading the title ("dragging [β¦]"), it is obvious! π¬
so it's unlikely to get missed for any length of time.
β¦ which is also true here: π Unable to add SDCs with slots to the page Active was reported on Jan 8, 15:49 CET, and it was introduced in π Throw exceptions instead of returning tuples for validation errors Active on Jan 7, 12:14 CET. Within 27.5 hours!
Instead, I wonder if we can find a way to programmatically call the same reducer function that occurs on the drop event directly from Cypress. This would ensure the same code runs (updating the JSON, sending the JSON to the server, receiving the updated HTML, updating the preview) without having to actually do all the fiddly stuff of emulating mouse moves and hover events etc.
So, a unit/kernel-esque test? If so: sounds perfect!
- πΊπΈUnited States hooroomoo
I am un-assigning myself from this issue for now and going to add an e2e test that is suggested in #4 in a separate issue π Add e2e test that adds a component with an empty slot to the canvas Active where I am going to add the component not with dragging but by using the "Add component" click to insert mechanism because adding that test should be easy and I agree with Ted would be worth having.
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Jesseβs call on where to take this after #7 :)
- πΊπΈUnited States tedbow Ithaca, NY, USA
I don't think is particularly valuable to have an end to end test that ACTUALLY drags and drops elements (e.g. mousedown, mousemove, mouseover, mouseup etc) . It is a lot of work, is prone to failure etc
@jessebaker and/or @hooroomoo are the same actions possible through keyboard actions. If so would this be easier to test?
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
π Add e2e test that adds a component with an empty slot to the canvas Active landed, what remains now?
- π¬π§United Kingdom jessebaker
RE: #14
I think this should be postponed until either
- π Create extendibility proof of concept that also serves as documentation-by-example Active lands and we can programmatically call Redux actions like "insertNodeAtPath" or "moveNodeToPath" or
- OR some kind of more accessible UX is implemented that allows e.g. components to be added to slots via clicking, without drag+drop (which is alluded to in this Figma or see screenshot below )