Closed darth-cheney closed 3 years ago
@darth-cheney this looks good and it feels a lot snappier. Some comments made above. I wasn't able to break it (yet) within an environment but importing a few worlds then serializing broke nav (it would open and close but no longer would responding to navigation clicks or any model-view updates)
no errors were thrown when de-serializing. I'll post an example in slack
copy and pasted elements don't appear in the nav
here is example in an area
but also true of any place you can paste (like a card)
also the dev tools message func is throwing errors. Do you want to just disable it here
i did this on my branch
we can come back to message related devtools, but as discussed it might be time to move all this debugging to ST itself
@darth-cheney
this all fixes up the issues above
i am still/now? seeing this one: opening script editors on any view doesn't appear in the nav card view, but does in the nav stack view. This is also true for the EditorView
custom component but that is b/c it's not an ST part (when we fix that up one way or another this will go away)
ah nevermind this is b/c the script editor is on the current stack not on the card. I fixed this up in the clean up PR #184
One small thing I noticed is that if I save a page with the navigator open, it doesn't stay open when I open that page. So I guess the state of the navigator pane isn't included in the serialization. I assume we don't care about this at all, but if it's a trivial thing to add it might be good since it makes the idempotency a little stronger. But super small and not too important.
One small thing I noticed is that if I save a page with the navigator open, it doesn't stay open when I open that page. So I guess the state of the navigator pane isn't included in the serialization. I assume we don't care about this at all, but if it's a trivial thing to add it might be good since it makes the idempotency a little stronger. But super small and not too important.
@ApproximateIdentity yeah this part of the design. Otherwise it won't load on de-serialization
What
This PR fixes display and logic bugginess in the Navigator.
Problems
There were several cascading problems, partially identified in #179 and #180, that were causing Navigator to completely bug-out and at times freeze the whole system.
There were a few things going on. First, flipping back and forth quickly between two Stacks with the Nav open could lead to display bugs. This is because the CardRow for each Stack was being composed and re-rendered on the fly each time, meaning a whole tree of elements was being copied, inserted, and having its model set (recursively) each time we switched between stacks.
The "layout pushes up" issue was a fairly arcane one. It only applied to
FieldView
and no other parts. It turns out thatFieldView
sets its element focus when it is attached to the DOM. Because the Navigator's "lensed views" are just PartView elements with the same model attached, the browser was attempting to focus the tiny, scaled-down field in the Nav, meaning it would scroll (without scrollbars) to that element to get it within the viewport. This is why things seemed to "push up"Fixes and Improvements
WrappedView
is now a PartViewPreviously, WrappedView was simply a custom webcomponent that had no strict relation to the rest of the SimpleTalk system. But now we've made the better decision to make it a PartView subclass, meaning it can accept a model and has all the lifecycle methods we would expect from a normal PartView.
Instead of waiting for some user action or for the right DOM to be set, the nested "lensed" trees will be created whenever the model is set on a WrappedView. This is also when the scaling of the view will occur. Consequently, "currentness" can now apply to the WrappedView as well, where needed.
Optimizing CardRow Rendering
As mentioned, we were re-rendering the CardRow WrappedViews each time we switched between Stacks. That's a lot of computation to happen if you are toggling back and forth quickly. The main Navigator PartView will now pre-render all CardRows for any Stack that is loaded into the System, showing only the CardRow that is mapped to the current stack.
The latter process is interesting: we simply set the
slot=
attribute on the<nav-card-row>
(a CardRow instance) that maps to the current stack, and we remove that attribute on all others. This results in an instant swapping, since CardRow must be specifically slotted in Navigator's shadow dom.The "pushing-up" issue
The solution here was crude but effective: remove the line where FieldView automatically gets the focus whenever it is attached to the DOM. We definitely don't want fields inside of the nav trying to get the focus. There might be a better way down the road to ensure that the main FieldViews do get the focus that we want while ensuring that WrappedView copies are not affected. TBD.
Toggling should now happen quickly and without bugginess.
Other Fixes
I've added some minor styling fixes here and there. For example, Cards that make use of a list-layout on themselves should now display correctly in the Nav WrappedViews.
What To Do
Steps to really put the Navigator through the ringer:
importWorld "layout-examples.html"
or whatever stack you want. Note that at this point in the branching any stack with a Drawing in it will not load properlyTests
We had to update some querySelectors in a few tests. These selectors assumed only a single stack/card tree existing in the document. But with Nav we can have copies of stacks and cards inside of the navigator. Some of the tests were pulling out the copies and not the main views to be tested. The solution is to prepend
st-world
to any relevant querySelector, which will then not include anything in the navigator.Closes
179
180