DynamicZone - extend FriendlyName with MainValue #12500

godzzo · 2022-02-13T06:42:24Z

...like RepeatableComponent

This commit fixes #12499

What does it do?

Show the MainValue of the Components of a DynamicZone in the AccordionToggle.

Why is it needed?

To be able to distingish the Dzone components without unfold them.

How to test it?

Create a Component
Create a Collection
- add DynamicZone
  - add the Component
Edit the Collection
- edit DZone
  - add more Component instance
  - edit the mainField
  - check the AccordionToggle changes

Sample:

Related issue(s)/PR(s)

Related issue: #12499

gu-stav · 2022-02-22T17:12:42Z

@godzzo Thanks for your contribution again :) We had a quick look into the PR today and think it's a nice addition. I'll check & review your code in the coming days.

gu-stav

As I already said: thank you for the contribution. I've added a couple of comments, but overall this is an addition we'd like to welcome.

...es/core/admin/admin/src/content-manager/components/DynamicZone/components/Component/index.js

...ges/core/admin/admin/src/content-manager/components/DynamicZone/components/Component/util.js

...es/core/admin/admin/src/content-manager/components/DynamicZone/components/Component/index.js

gu-stav · 2022-02-28T16:19:29Z

...ges/core/admin/admin/src/content-manager/components/DynamicZone/components/Component/util.js

+import { get, toString } from 'lodash';
+import { useCMEditViewDataManager } from '@strapi/helper-plugin';
+
+function useMainValue(schema, componentFieldName) {


Could you add a test for this file?

I am not familiar the testing in this project. Could you give me some docs and an example?

I think the best would be to copy this file into a utils folder (e.g. ./utils/useMainValue.js. Then you can add a test folder within utils and write a basic unit-test similar to this one, which tests input / output.

I have decoupled the displayedValue logic and pushed a unit test file for it.

In Win10 the jest could not find any test file, I tested on WSL2-Ubuntu.

testMatch: /**/tests/**/?(*.)+(spec|test).[jt]s?(x) - 0 matches

codecov · 2022-03-01T20:09:51Z

Codecov Report

Base: 59.33% // Head: 59.33% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (ab1d9e6) compared to base (a13b60d).
Patch coverage: 58.82% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #12500      +/-   ##
==========================================
- Coverage   59.33%   59.33%   -0.01%     
==========================================
  Files        1338     1342       +4     
  Lines       32724    32757      +33     
  Branches     6197     6199       +2     
==========================================
+ Hits        19417    19436      +19     
- Misses      11438    11450      +12     
- Partials     1869     1871       +2

Flag	Coverage Δ
front	`63.63% <58.82%> (-0.01%)`	⬇️
unit	`49.80% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...s/DynamicZone/components/Component/utils/select.js	`36.36% <36.36%> (ø)`
...micZone/components/Component/hooks/useMainValue.js	`58.33% <58.33%> (ø)`
.../DynamicZone/components/Component/utils/connect.js	`66.66% <66.66%> (ø)`
...mponents/DynamicZone/components/Component/index.js	`47.54% <100.00%> (+1.77%)`	⬆️
...ts/DynamicZone/components/Component/utils/index.js	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

gu-stav

Thanks again @godzzo. I've added two small comments. It would still be nice to have tests for useMainValue(). You can find a way to test hooks here: https://github.com/strapi/strapi/pull/12893/files#diff-9530f953e7c9e1ef6b200896b0fbcb9ee54c81ff875bbae6dc61b080e2c10d7f , but since the tests seem to create problems on your machine, you can let me know if you want me to push a test for it?

...es/core/admin/admin/src/content-manager/components/DynamicZone/components/Component/index.js

.../admin/src/content-manager/components/DynamicZone/components/Component/utils/useMainValue.js

soupette · 2022-03-29T08:38:53Z

...es/core/admin/admin/src/content-manager/components/DynamicZone/components/Component/index.js

+
+    return layout;
+  }, [componentUid, getComponentLayout]);
+  const mainValue = useMainValue(componentLayoutData, [name, index]);


I would recommend to pass the mainValue as a prop from the DynamicZone/utils/select.js hook to avoid extreme rerendering of the components, it can cause performance issues at some point.

I have created a new wrapper with utils/connect for DynamicZone/Component.

Please check this out. It is okay for using this separately for each Component?
(it feel cleaner for me and the RepeatableComponent/DraggedItem using the same way, if I see it correctly).

Or should I move the mainValue parsing here DynamicZone/utils/select.js:

const dynamicDisplayedComponents = useMemo( () => get(modifiedData, [name], []).map(data => data.__component), [modifiedData, name] );

.../admin/src/content-manager/components/DynamicZone/components/Component/hooks/useMainValue.js

strapi-bot · 2022-05-05T12:32:45Z

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/component-entry-titel-display-in-dynamic-zone/15478/3

sjaga003 · 2022-05-23T23:25:52Z

Any chance this will get reviewed and merged? Would love to be able to use this feature

gu-stav · 2022-05-24T07:12:26Z

@sjaga003 I'll do another review as soon as possible - I think I'll manage it by next week.

Zharkan · 2022-06-07T16:12:49Z

@sjaga003 I'll do another review as soon as possible - I think I'll manage it by next week.

Nice, we need this.

derrickmehaffy · 2022-08-08T17:21:15Z

Hey all, let me request and update from the team

gu-stav · 2022-08-08T17:25:51Z

@derrickmehaffy I'm on it. The required design-System work is done, but I have to find some time to review this PR again.

derrickmehaffy · 2022-08-08T19:24:43Z

@derrickmehaffy I'm on it. The required design-System work is done, but I have to find some time to review this PR again.

No problem thank you!

gu-stav · 2022-08-23T08:27:44Z

Hello 👋🏼 I've checked the PR again today, because now all required changes to the Accordion component are done, so that long titles don't overflow any longer.

I've found a couple of bugs:

only fields of type "text" as main-field are extracted properly; all the others lead to 1
all fields of type !== 'text' update on save, whereas type == 'text' updates on keydown
if no value was extracted, the seperator - is still rendered

Would you still be available to fix those?

godzzo · 2022-11-04T12:47:14Z

Hello @gu-stav !

Yes! I have tried to fix the 3 bugs you mentioned.

Please review it.

only fields of type "text" as main-field are extracted properly; all the others lead to 1
- if it is not a title field (you could not choose as title of the DZone) then it will be the id field
- that is why 1 is the value (of id) if you save that...
all fields of type !== 'text' update on save, whereas type == 'text' updates on keydown
- id only got value if you save, otherwise will be update on keydown if I saw it correctly
- I omit the id as MainField value, that is the fix about
if no value was extracted, the seperator - is still rendered
- fixed

…nent This commit fixes strapi#12499

…ieldName (string) parameter reorged to componentFieldPath (array)

…ue and add a unit test for it - without call the test.

…the proper hooks/ folder.

…icZone/Component

Co-authored-by: Gustav Hansen <gu@stav.dev>

…eed separator.

gu-stav

It works well on my side now. 🚀 Thanks for staying on it and fixing all the problems!

gu-stav added the source: core:content-manager Source is core/content-manager package label Feb 14, 2022

gu-stav self-assigned this Feb 22, 2022

gu-stav reviewed Feb 28, 2022

View reviewed changes

alexandrebodin added the pr: fix This PR is fixing a bug label Mar 9, 2022

alexandrebodin requested a review from gu-stav March 9, 2022 16:14

gu-stav added pr: enhancement This PR adds or updates some part of the codebase or features and removed pr: fix This PR is fixing a bug labels Mar 10, 2022

gu-stav reviewed Mar 25, 2022

View reviewed changes

...es/core/admin/admin/src/content-manager/components/DynamicZone/components/Component/index.js Outdated Show resolved Hide resolved

.../admin/src/content-manager/components/DynamicZone/components/Component/utils/useMainValue.js Outdated Show resolved Hide resolved

soupette reviewed Mar 29, 2022

View reviewed changes

gu-stav reviewed Mar 31, 2022

View reviewed changes

.../admin/src/content-manager/components/DynamicZone/components/Component/hooks/useMainValue.js Outdated Show resolved Hide resolved

gu-stav self-requested a review April 4, 2022 07:36

This comment was marked as spam.

Sign in to view

gu-stav mentioned this pull request Jul 14, 2022

Accordion: Make height of AccordionToggle flexbile and add title ellipsis strapi/design-system#652

Merged

This comment was marked as spam.

Sign in to view

gu-stav force-pushed the godzzo/issue12499 branch from 62c179f to 5e8d63b Compare August 23, 2022 08:02

derrickmehaffy mentioned this pull request Sep 12, 2022

DynamicZone - extend FriendlyName with MainValue like RepeatableComponent #12499

Closed

2 tasks

gu-stav force-pushed the godzzo/issue12499 branch from 082ec47 to 1404123 Compare November 4, 2022 13:08

godzzo and others added 8 commits November 11, 2022 13:03

DynamicZone - extend FriendlyName with MainValue like RepeatableCompo…

469ff7e

…nent This commit fixes strapi#12499

console.log removed, mainValue length limited to 50 chars, componentF…

7afa200

…ieldName (string) parameter reorged to componentFieldPath (array)

decouple the getDisplayedValue logic for unit testing from useMainVal…

600df90

…ue and add a unit test for it - without call the test.

Remove length limit from displayedValue, and move useMainValue.js to …

20f7fed

…the proper hooks/ folder.

adding wrapper (connect, select) for using mainValue as prop in Dynam…

56607b6

…icZone/Component

separate imports for better tree shaking

20ccfeb

Co-authored-by: Gustav Hansen <gu@stav.dev>

fixing 'id' mainField (no title field), and empty display value not n…

46851af

…eed separator.

fixing format issues (lint (node: 18))

9b2d5b0

gu-stav force-pushed the godzzo/issue12499 branch from 1404123 to 9b2d5b0 Compare November 11, 2022 12:03

Chore: Fix eslint errors

ab1d9e6

gu-stav approved these changes Nov 11, 2022

View reviewed changes

gu-stav merged commit e68bca1 into strapi:main Nov 11, 2022

gu-stav added this to the 4.5.1 milestone Nov 11, 2022

gu-stav mentioned this pull request Nov 14, 2022

[DynamicZone]: Cleanup & test #14850

Merged

Uh oh!

DynamicZone - extend FriendlyName with MainValue #12500

DynamicZone - extend FriendlyName with MainValue #12500

Uh oh!

Conversation

godzzo commented Feb 13, 2022

What does it do?

Why is it needed?

How to test it?

Related issue(s)/PR(s)

Uh oh!

gu-stav commented Feb 22, 2022

Uh oh!

gu-stav left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gu-stav Feb 28, 2022

Choose a reason for hiding this comment

Uh oh!

godzzo Mar 1, 2022

Choose a reason for hiding this comment

Uh oh!

gu-stav Mar 2, 2022

Choose a reason for hiding this comment

Uh oh!

godzzo Mar 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Mar 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gu-stav left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

soupette Mar 29, 2022

Choose a reason for hiding this comment

Uh oh!

godzzo Mar 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

strapi-bot commented May 5, 2022

Uh oh!

sjaga003 commented May 23, 2022

Uh oh!

gu-stav commented May 24, 2022

Uh oh!

Zharkan commented Jun 7, 2022

Uh oh!

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

derrickmehaffy commented Aug 8, 2022

Uh oh!

gu-stav commented Aug 8, 2022

Uh oh!

derrickmehaffy commented Aug 8, 2022

Uh oh!

gu-stav commented Aug 23, 2022

Uh oh!

godzzo commented Nov 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gu-stav left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

godzzo Mar 3, 2022 •

edited

Loading

codecov bot commented Mar 1, 2022 •

edited

Loading

godzzo Mar 30, 2022 •

edited

Loading

godzzo commented Nov 4, 2022 •

edited

Loading