feat(ui): create DescriptionList with DescriptionTerm and DescriptionDefinition #1329
Conversation
guoda-puidokaite
added
package
|
|
guoda-puidokaite
changed the title
|
Would you please add the link to the original issue to make it easier to look up requirements and acceptance criteria? |
There was a problem hiding this comment.
While we used to add stories for all child components of a parent component, I think DL with DT and DD is a good demonstration case why/when we shouldn't: <dt> and <dd>. elements really do not make any sense to use outside of a <dl> and represent structurally and semantically invalid html, we shouldn't demonstrate their standalone use in storybook as it could lead users to think it may be possible or valid to use them standalone. Same goes for DD.
IMO we should concentrate n showing real-world-like use cases in the DL story, demonstrating good, semantic and valid use of all three components together, and get rid of the individual child component stories.
There was a problem hiding this comment.
For idinvidual DD stories, the same applies as to DT. These components do not make any sense to use alone / ouside of a DL parent (nd may be invalid html), and we shouldn't demonstrate such a use IMO.
There was a problem hiding this comment.
Would be good to have a story with more DT and DD pairs with some more real-world-like metadata content to better illustrating their intended use case. Maybe we can supply some content, @edda ?
| * Pairs with DescriptionTerm to complete the term-description association, offering flexible content styling. | ||
| */ | ||
| export const DescriptionDefinition: React.FC<DescriptionDefinitionProps> = ({ children, className = "" }) => ( | ||
| <div className="dd-container"> |
There was a problem hiding this comment.
There is no need to wrap each <dd> in a container <div>, this is most likely even invalid.
The ticket mentions an option to allow users to wrap one DT and one or multiple DD elements in a <div> for better styling flexibility as outlined in the spec here:
| * Used to denote terms, headers, or keys in a semantic way, allowing for flexible styling. | ||
| */ | ||
| export const DescriptionTerm: React.FC<DescriptionTermProps> = ({ children, className = "" }) => ( | ||
| <div className="dt-container"> |
There was a problem hiding this comment.
Please don't wrap the <dt> in a container div, see https://github.com/cloudoperators/juno/pull/1329/files#r2538718640, the exact same applies here.
| gap: 0.25rem; | ||
| } | ||
|
|
||
| .dd { |
There was a problem hiding this comment.
Both <dt> and <dd> should have a bottom border as specified in the ticket.
Updated the variable table in the ticket to map a new variable to the existintheme- variable.
| gap: 0.25rem; | ||
| } | ||
|
|
||
| .dt { |
There was a problem hiding this comment.
Both <dt> and <dd> should have a bottom border as specified in the ticket.
Updated the variable table in the ticket to map a new variable to the existintheme- variable.
franzheidl
left a comment
franzheidl
left a comment
There was a problem hiding this comment.
See individual comments,.
guoda-puidokaite
changed the title
Summary
Introduce a standardised
DescriptionListcomponent along with its child components,DescriptionTermandDescriptionDefinition, to display terms and descriptions consistently across applications following semantically correct HTML elements.Changes Made
DescriptionList,DescriptionTerm, andDescriptionDefinitionComponents using corresponding html elements.<dt>, supporting valid spec structure and potential future layouts using grid systems.Testing Instructions
Check ACs, Figma and test in Storybook.
Checklist
PR Manifesto
Review the PR Manifesto for best practises.