Fix sidebar loading causes UI jump (#15719)

* Fix sidebar loading causes UI jump

* Fix sidebar blinks betweed data refreshes

* Replace `...` with `…` character

* Test collection is open immediately after moving

* Update sidebar children cypress selector

* Add comment about sidebar's loadingAndErrorWrapper

* Fix typo

* Add role props to sidebar and its children

frontend/src/metabase/collections/components/CollectionsList.jsx

@@ -42,11 +42,13 @@ class CollectionsList extends React.Component {
                       onClick={() => c.children && action(c.id)}
                       hovered={hovered}
                       highlighted={highlighted}
+                      role="treeitem"
+                      aria-expanded={isOpen}
                     >
                       <Flex
                         className="relative"
                         align={
-                          // if a colleciton name is somewhat long, align things at flex-start ("top") for a slightly better
+                          // if a collection name is somewhat long, align things at flex-start ("top") for a slightly better
                           // visual
                           c.name.length > 25 ? "flex-start" : "center"
                         }

frontend/src/metabase/collections/containers/CollectionSidebar.jsx

@@ -11,6 +11,7 @@ import Collection from "metabase/entities/collections";
 
 import Icon from "metabase/components/Icon";
 import Link from "metabase/components/Link";
+import LoadingSpinner from "metabase/components/LoadingSpinner";
 
 import CollectionsList from "metabase/collections/components/CollectionsList";
 import CollectionLink from "metabase/collections/components/CollectionLink";
@@ -41,16 +42,22 @@ const Sidebar = styled(Box)`
     we should eventually refactor code elsewhere in the app to use this by default instead of determining the relationships clientside, but this works in the interim
   */
   query: () => ({ tree: true }),
+
+  // Using the default loading wrapper breaks the UI,
+  // as the sidebar has a unique fixed left layout
+  // It's disabled, so loading can be displayed appropriately
+  // See: https://github.com/metabase/metabase/issues/14603
+  loadingAndErrorWrapper: false,
 })
 class CollectionSidebar extends React.Component {
   state = {
     openCollections: [],
   };
 
-  componentDidMount() {
-    // an array to store the ancestors
+  componentDidUpdate(prevProps) {
     const { collectionId, collections, loading } = this.props;
-    if (!loading) {
+    const loaded = prevProps.loading && !loading;
+    if (loaded) {
       const ancestors = getParentPath(collections, Number(collectionId)) || [];
       this.setState({ openCollections: ancestors });
     }
@@ -71,10 +78,10 @@ class CollectionSidebar extends React.Component {
   // TODO Should we update the API to filter archived collections?
   filterPersonalCollections = collection => !collection.archived;
 
-  render() {
+  renderContent = () => {
     const { currentUser, isRoot, collectionId, list } = this.props;
     return (
-      <Sidebar w={340} pt={3} data-testid="sidebar">
+      <React.Fragment>
         <CollectionLink
           to={Urls.collection("root")}
           selected={isRoot}
@@ -126,6 +133,23 @@ class CollectionSidebar extends React.Component {
             {t`View archive`}
           </Link>
         </Box>
+      </React.Fragment>
+    );
+  };
+
+  render() {
+    const { allFetched } = this.props;
+
+    return (
+      <Sidebar w={340} pt={3} data-testid="sidebar" role="tree">
+        {allFetched ? (
+          this.renderContent()
+        ) : (
+          <div className="text-brand text-centered">
+            <LoadingSpinner />
+            <h2 className="text-normal text-light mt1">{t`Loading…`}</h2>
+          </div>
+        )}
       </Sidebar>
     );
   }

frontend/test/metabase/scenarios/collections/collections.cy.spec.js

@@ -388,18 +388,11 @@ describe("scenarios > collection_defaults", () => {
         "**New collection should immediately be open, showing nested children**",
       );
 
-      openDropdownFor(NEW_COLLECTION);
-      cy.findAllByText("First collection");
-      cy.findAllByText("Second collection");
-
-      // TODO: This was an original test that made sure the collection is indeed open immediately.
-      //       That part is going to be addressed in a separate issue.
-      // cy.findByText(NEW_COLLECTION)
-      //   .closest("a")
-      //   .within(() => {
-      //     cy.icon("chevrondown");
-      //     cy.findByText("First collection");
-      //   });
+      getSidebarCollectionChildrenFor(NEW_COLLECTION).within(() => {
+        cy.icon("chevrondown").should("have.length", 2); // both target collection and "First collection" are open
+        cy.findByText("First collection");
+        cy.findByText("Second collection");
+      });
     });
 
     it("should update UI when nested child collection is moved to the root collection (metabase#14482)", () => {
@@ -590,3 +583,12 @@ function selectItemUsingCheckbox(item, icon = "table") {
         .click();
     });
 }
+
+function getSidebarCollectionChildrenFor(item) {
+  return cy
+    .findByTestId("sidebar")
+    .findByText(item)
+    .closest("a")
+    .parent()
+    .parent();
+}