customfields: allow hidding section name #248
Conversation
jrcastro2
commented
Jul 31, 2024
jrcastro2
commented
- closes SSPN: vocabularies CERNDocumentServer/cds-rdm#143
jrcastro2
force-pushed
the
allow-hide-accordion-custom-fields
branch
from
a3ed259 to
0c47819
Compare
| const { fields, paths, section: sectionName = "" } = section; | ||
|
|
||
| return sectionName === "" ? ( | ||
| <Container key={`section-${sectionName}`}>{fields}</Container> |
There was a problem hiding this comment.
I would rather have config key e.g visible, hide to control this... With the current solution the key also is wrong as the sectionName is an empty string no?
There was a problem hiding this comment.
Hmm that's was my first thought and I started by doing that but then I realised that if we don't add the section it will display wronlgy, like bugged accordion, and since when we hide it, the section name is not used at all I thought that it could help to fix the bug and simplify the logic. If section name is empty the key will be simply section-, it's not wrong simply a generic key. If you think there is added value to use the config key, I can add it
There was a problem hiding this comment.
I wasn't thinking of the flag show or hide the accordion but rather just being used as part of your condition check i.e instead of if sectionName === ''
There was a problem hiding this comment.
Unless I am missing something, that is exactly what is used to show or hide the accordion, feel free to reach out if you think it might be faser to discuss IRL!
There was a problem hiding this comment.
I have another question: why was this change needed?
There was a problem hiding this comment.
In CDS we want to add the custom fields (experiment and department) inside the basic information section, meaning that they are not part of their own section, we need to be able to add fields without section
There was a problem hiding this comment.
I would then try to find some other solution for this, because passing the section as empty string seems like a temporary workaround that should not make it inside the code. Can we do a demo?
| </AccordionField> | ||
| ))} | ||
| sections.map((section) => { | ||
| const { fields, paths, section: sectionName = "" } = section; |
There was a problem hiding this comment.
I think what Zach might be suggesting is
| const { fields, paths, section: sectionName = "" } = section; | |
| const { fields, paths, section: sectionName, hidden = False } = section; |
I also think comparison with empty string to determine if the section is supposed to be displayed is not intuitive
jrcastro2
force-pushed
the
allow-hide-accordion-custom-fields
branch
2 times, most recently
from
1cb72f8 to
4af4470
Compare
jrcastro2
force-pushed
the
allow-hide-accordion-custom-fields
branch
from
4af4470 to
dab23e8
Compare
