-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Docs2: Refactor manager to use new index data #18023
Conversation
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 349ba7b. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this branch ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
f9698f3
to
95feebc
Compare
…o future/docs2/manager-ui
…o future/docs2/manager-ui
}); | ||
}); | ||
|
||
it('collapses mixtures of leaf and non-leaf children', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This tested an impossible situation -- you cannot have a story as a child of a root. Perhaps there was something legitimate this was trying to test, I am not sure. Aiming to revisit DOCS_MODE
later.
For forwards compatibility with changes coming in 7.0, see: #18023
…o future/docs2/manager-ui
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tmeasday looks like there are a bunch of unintentional changes in chromatic. can you please take a look? 🙏
The numeric indexes meant that the keys were re-ordered, which broke things.
@shilman the 3 remaining Chromatic changes exist on the base branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing job @tmeasday !!! Merging 🎉
Previously,
We massaged data in three places:
When receiving a set of stories either from
SET_STORIES
(v6) orstories.json
(v7), we denormalized it (inapi/lib/stories.ts
) by creating entries in thestoriesHash
for each root, group and component that contained a story.i. We represented "docs nodes" in this representation as stories with
parameters.docsOnly = true
ii. As the story index didn't contain parameters, we did a hack/heuristic to check if a story was the only child of a component and had the name
Page
, and prefilledparameters.docsOnly
if so.When rendering the sidebar (in
ui/components/data.ts
), we "collapse" stories into their components in the following ways:i. For normal mode, we'd pick out docs only stories (based on the
parameters.docsOnly
) and collapse them into their containing component, ending up with an entry in the hash withisComponent = isLeaf = true
and a story id.ii. For
DOCS_MODE
, we'd collapse the first story of every component into it, as above, and delete all the other stories.ui/components/sidebar/Tree.tsx
), we'd:ii. Move "unrooted" components and groups to the front of the hash
ii. Collapse the data another time to deal with "single story components" -- components with a single contained story that has the same name as the component. collapse the data another time to deal with "single story components" -- components with a single contained story that has the same name as the component.
My thinking in this PR is that there are two problems with the above:
A. We no longer represent docs entries in the index as
parameters.docsOnly
stories, so we can get away with doing the same in thestoriesHash
and make a much clearer distinction between story entries (which can be rendered in both view modes, currently) and docs entries (which can only be rendered in docs view mode).B. The reason we denormalize into
storiesHash
(and don't pass around theStoryIndex
data structure) is to make our life easier when doing things likegoToNextComponent
etc. However that breaks down if thestoriesHash
doesn't actually represent what we are rendering in the sidebar (which is what happens when doing later collapsing in 2&3).For instance, you cannot sensibly navigate to "unrooted" stories with the keyboard shortcuts, because the position they appear in the list is different to the internal representation.
So the solution appears to be to do all the "massaging" initially when creating the
storiesHash
Considering "Single Story Components":
There are three cases were we want to combine or collapse a component and its child(ren):
DOCS_MODE
.When we collapse a component into its children, we want to make the component:
We have a choice here, we could either component the
ComponentEntry
andStoryEntry
(orDocsEntry
) into a single entry (of a new type), or we could simply render the two entries as a single node in the tree.To decide, we should think carefully about how we want keyboard navigation to work. Suppose we have three stories:
The second component is a "single story" component (the other two are not as they have different names from their only story). How should keyboard navigation work?
a
, "next story" should take you tob
, "next component" should take you tob
alsob
, "next story" should take you toc
, "next component" should take you toc
alsoIn this way, we want to act on the has as if the original structure was there.
In this PR,
The types for the
HashEntry
s (in theStoryHash
) are simplified via the use of atype
field and separated cleanly intoRootEntry
,GroupEntry
,ComponentEntry
,DocsEntry
andStoryEntry
.When rendering in normal mode, a component entry with a single docs entry is created for docs pages.
When creating the
storiesHash
, the unrooted stories are sorted to the front automatically.When rendering in-- I decided to this in a future PR, as it wasn't strictly necessary for the other changes, just related.DOCS_MODE
, we filter out all non-first children when creating the hash (1 above), and rename the first child to the name of the component.When rendering the tree, we render the combination of a
ComponentEntry
and its "single story" (see above) as a singleStoryNode
orDocsNode
.