strapi icon indicating copy to clipboard operation
strapi copied to clipboard

DynamicZone - extend FriendlyName with MainValue

Open godzzo opened this issue 3 years ago • 17 comments

...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: strapi_dzone_main_value

Related issue(s)/PR(s)

Related issue: #12499

godzzo avatar Feb 13 '22 06:02 godzzo

@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 avatar Feb 22 '22 17:02 gu-stav

Codecov Report

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

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%) :arrow_down:
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%) :arrow_up:
...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.

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

codecov[bot] avatar Mar 01 '22 20:03 codecov[bot]

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

strapi-bot avatar May 05 '22 12:05 strapi-bot

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

sjaga003 avatar May 23 '22 23:05 sjaga003

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

gu-stav avatar May 24 '22 07:05 gu-stav

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

Nice, we need this.

Zharkan avatar Jun 07 '22 16:06 Zharkan

review + merge, pretty please 🙏

Sija avatar Jun 24 '22 18:06 Sija

We look forward to this addition :pray:

tpapamichail avatar Jun 28 '22 11:06 tpapamichail

Any update on this review? Its a necessary addition 🙏

mohdtaha60 avatar Jul 13 '22 05:07 mohdtaha60

This is the main pain point for our editors. Please give it another look.

Torsten85 avatar Jul 29 '22 11:07 Torsten85

This is the main pain point for our editors. Please give it another look.

You can use patch package to fix this for now. We've been doing this to accommodate the editors.

Zharkan avatar Jul 29 '22 12:07 Zharkan

Any update on this getting merged in?

sjaga003 avatar Aug 08 '22 16:08 sjaga003

Hey all, let me request and update from the team

derrickmehaffy avatar Aug 08 '22 17:08 derrickmehaffy

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

gu-stav avatar Aug 08 '22 17:08 gu-stav

@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!

derrickmehaffy avatar Aug 08 '22 19:08 derrickmehaffy

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
Screenshot 2022-08-23 at 10 19 11

Would you still be available to fix those?

gu-stav avatar Aug 23 '22 08:08 gu-stav

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

godzzo avatar Nov 04 '22 12:11 godzzo