feat: show hidden fields in history frontend

[remidej]

What does it do?

Makes hidden fields visible in the History page. This is not about internal fields like id, but about the ones the user may have deleted from the layout via the configure the view page. I called them "remaining fields" in the code, to match what was already done in the code for that page.

It applies the fix in two places:

• At the root of the history page layout. This one was the most straight forward.
• Inside the components. We didn't have direct access to the component's rendering logic, so I had to refactor it to pass a new renderLayout. It has the benefit of avoiding duplicating logic between repeatable and non-repeatable components, and avoids some prop drilling for renderInput.

I want to write e2e tests for this, they have issues we need to fix first.

Why is it needed?

Because even though they're not visible in the edit view, the fields that aren't in the layout would still be restored if the history version is restored. It's therefore important to show them regardless, so that users know what they're restoring.

How to test it?

Go to the configure the view page for a content type. Delete a root field there (not in the CTB), as well as a field that's inside a component.

Now open the history page for an entry of that content type. You should see the deleted root field on a new line at the bottom of the page, but above the unknown fields.

Also check that component inputs work as expected in the Edit View page, since their code was refactored.

[joshuaellis]

If you need to change the entire layout, which is basically the child of the strapi component field, doesn't it just make more sense to enable children to be passed? We'd then export the component without children applied, it could be moved up to a more generic domain e.g. components and in the EditView we add the current layout and in history we add your revised layout requirements?

How did we come to the conclusion this was the right solution? I'm just trying to weigh up the right direction in the long term... 🤔

[markkaylor]

I had this error:

I'm not sure exactly how it would work with you remaining fields function but I'm curious what you think of using children instead of two render props. I believe this is working and accomplishes the same thing: https://github.com/strapi/strapi/pull/20207/files

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
contributor-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 30, 2024 3:59pm

[remidej]

__temp_key__ is undefined in history, I'm not sure how it can be defined in a proper way (or it is matters)

[joshuaellis]

I'd like to avoid creating keys like this if possible, we had this in V4 and it got very messy and quite buggy to follow, especially if it leaks into the EditView, the re-ordering will crash.

We create tmp keys when we fetch the document:

strapi/packages/core/content-manager/admin/src/pages/EditView/EditViewPage.tsx

Line 122 in 4300e8f

const initialValues = React.useMemo(() => {

strapi/packages/core/content-manager/admin/src/pages/EditView/utils/data.ts

Line 127 in 4300e8f

const prepareTempKeys = traverseData(

You could use the same pipe funcs perhaps? or maybe just the tmp key one.

[remidej]

@joshuaellis I think I see what you're saying, but with the current state solution I think I'm happy where things stand:

• HistoryPage fetches the layout data. I like that because that component centralizes all the loading and error states, so we have one single Loader
• VersionContent transforms the layout and renders it. It's the only purpose of the component so I feel like it's pretty readable
• VersionInputRenderer does a little big of logic to compute the layout for components

For components I agree it could be nice to not have some layout logic in the input renderer for components. But I feel like extracting things to a hook will add more complexity rather than simplify things 🤔

[markkaylor]

Thanks for trying several approaches to find what will work best for us 🙏 . I like what you have here.

• I have this error when trying to access history on a content type with a component
  
• I find it a bit strange that hidden fields don't have a section title like unknown fields. Isn't it a bit confusing for the user to just see some fields in a different panel? Maybe we should ask Lucas?

[remidej]

@markkaylor thanks for bug report! I was only checking with content types that had hidden fields.

About the hidden fields section. I thought about that but I'm not sure it's a good idea. I think it's important that unkown fields remain clearly different from the rest, because they're the ones that actually behave differently, as they can't be restored. If we have three different sections, we lose a bit of the clear page distinction between restorable and non-restorable. So I'm afraid trying to over-explain things by would actually create confusion.

There's also the issue of hidden fields inside of components. You can't extract them into their own section of the page, since they would lose their context. So you'd have to split each component into a known and an unknown section. It's not something I'm excited about.

We can also check with Lucas when he's back, but he's OOO until the Barcelona

[markkaylor]

There's also the issue of hidden fields inside of components. You can't extract them into their own section of the page, since they would lose their context. So you'd have to split each component into a known and an unknown section. It's not something I'm excited about

I think I see what you mean here and that does sound like a bummer. I noticed if I hide a field in a component via configure the view, then in the EditView it's hidden but in the history version it's just still there in the component. On the other hand if I hid the entire component via configure the view it shows up in the panel with the other hidden fields as expected. I guess that's the kind of thing you are talking about here and that's tomorrow's problem?

• I found one last error when finishing my QA when trying to access history of a content type that does not have a component:
  

Besides that I'm good to approve 👍

[joshuaellis]

Great job sticking with it to find the best API we can for the component stuff 👏🏼 I've not QAd but code makes sense ⭐

[remidej]

I found one last error when finishing my QA when trying to access history of a content type that does not have a component:

@markkaylor I can't reproduce this, could you tell me which content type you tested with? I think tried content types with and without components, with and without data in them, with and without hidden fields

I noticed if I hide a field in a component via configure the view, then in the EditView it's hidden but in the history version it's just still there in the component. On the other hand if I hid the entire component via configure the view it shows up in the panel with the other hidden fields as expected. I guess that's the kind of thing you are talking about here and that's tomorrow's problem?

I believe both cases are what we want, what is the issue you're seeing? Seeing a hidden field within a component is the reason I'm doing this whole refactoring. And for the hidden component there's no other choice but to show it at the bottom

[markkaylor]

So for components, I can hide a component and it works as expected:

Screen.Recording.2024-04-30.at.17.34.20.mov

But if I try and hide a field inside a component, then in the EditView it's hidden but in history it is not represented as a hidden field.

Screen.Recording.2024-04-30.at.17.33.17.mov

Let me know if it's clear or if you prefer to take a look on discord or something.

For the error, I had it on the Tag content type but I am unable to reproduce now too. Maybe it was stale data or something 🤷‍♂️