-
Notifications
You must be signed in to change notification settings - Fork 3
[CSL-3088] - fix issue with conflicting styles #128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 11 commits
d498f72
2f0d358
78d36b8
fb14b01
39167f7
8e4f80c
30b9294
b973dc8
8c4dfb5
69ca25f
42abce1
c980f9f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| /* eslint-disable no-param-reassign */ | ||
| import React from 'react'; | ||
| import useCioAutocomplete from '../../../hooks/useCioAutocomplete'; | ||
| import { isRecommendationsSection } from '../../../typeGuards'; | ||
| import { isCustomSection, isRecommendationsSection } from '../../../typeGuards'; | ||
| import { Item } from '../../../types'; | ||
| import { getStoryParams, toKebabCase } from '../../../utils'; | ||
| import { camelToStartCase, getStoryParams, toKebabCase } from '../../../utils'; | ||
|
|
||
| export function HooksTemplate(args) { | ||
| const { | ||
|
|
@@ -119,32 +119,41 @@ export function HooksTemplate(args) { | |
| if (!section?.data?.length) { | ||
| return null; | ||
| } | ||
|
|
||
| const { type, displayName } = section; | ||
| let sectionName = section.displayName; | ||
| const sectionListingType = isCustomSection(section) | ||
| ? 'custom' | ||
| : toKebabCase(section.indexSectionName || section.data[0]?.section || 'Products'); | ||
|
|
||
| let sectionTitle: string; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same for sectionTitle. Is there a way we can get this from getSectionProps instead? |
||
| switch (type) { | ||
| case 'recommendations': | ||
| sectionName = section.podId; | ||
| sectionTitle = section.podId; | ||
| break; | ||
| case 'custom': | ||
| sectionName = toKebabCase(displayName); | ||
| sectionTitle = displayName; | ||
| break; | ||
| case 'autocomplete': | ||
| sectionName = section.indexSectionName; | ||
| sectionTitle = section.indexSectionName; | ||
| break; | ||
| default: | ||
| sectionName = section.indexSectionName; | ||
| sectionTitle = section.indexSectionName; | ||
| break; | ||
|
Comment on lines
139
to
141
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is (type) broken currently but we still need it. Sections can be one of type There's two ways to fix this:
I'll pull up a separate PR for this.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. which part you mean "This is (type) broken currently"? @Mudaafi
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mocca102 Might be my configuration, but since we've explicitly defined the type |
||
| } | ||
|
|
||
| const recommendationsSection = isRecommendationsSection(section) | ||
| ? section.indexSectionName | ||
| : ''; | ||
| if (displayName) { | ||
| sectionTitle = displayName; | ||
| } | ||
|
|
||
| let sectionClassNames = toKebabCase(sectionListingType); | ||
| if (isRecommendationsSection(section)) { | ||
| sectionClassNames += ` ${toKebabCase(section.podId)}`; | ||
| } | ||
|
|
||
| return ( | ||
| <div key={sectionName} className={`${sectionName} ${recommendationsSection}`}> | ||
| <div key={sectionTitle} className={sectionClassNames}> | ||
| <div {...getSectionProps(section)}> | ||
| <h5 className='cio-sectionName'>{section.displayName || sectionName}</h5> | ||
| <h5 className='cio-section-name'>{camelToStartCase(sectionTitle)}</h5> | ||
| <div className='cio-section-items'> | ||
| {section?.data?.map((item) => renderItem(item))} | ||
| </div> | ||
|
|
@@ -271,32 +280,38 @@ function YourComponent() { | |
| if (!section?.data?.length) { | ||
| return null; | ||
| } | ||
|
|
||
| const { type, displayName } = section; | ||
| let sectionName = section.displayName; | ||
| let sectionTitle: string; | ||
|
|
||
| switch (type) { | ||
| case 'recommendations': | ||
| sectionName = section.podId; | ||
| sectionTitle = section.podId; | ||
| break; | ||
| case 'custom': | ||
| sectionName = toKebabCase(displayName); | ||
| sectionTitle = displayName; | ||
| break; | ||
| case 'autocomplete': | ||
| sectionName = section.indexSectionName; | ||
| sectionTitle = section.indexSectionName; | ||
| break; | ||
| default: | ||
| sectionName = section.indexSectionName; | ||
| sectionTitle = section.indexSectionName; | ||
| break; | ||
| } | ||
|
|
||
| const recommendationsSection = isRecommendationsSection(section) | ||
| ? section.indexSectionName | ||
| : ''; | ||
| if (displayName) { | ||
| sectionTitle = displayName; | ||
| } | ||
|
|
||
| let sectionClassNames = toKebabCase(sectionTitle); | ||
| if (isRecommendationsSection(section)) { | ||
| sectionClassNames += \` \${toKebabCase(section.indexSectionName)}\`; | ||
| } | ||
|
|
||
| return ( | ||
| <div key={sectionName} className={\`\${sectionName} \${recommendationsSection}\`}> | ||
| <div key={sectionTitle} className={sectionClassNames}> | ||
| <div {...getSectionProps(section)}> | ||
| <h5 className='cio-sectionName'>{sectionName}</h5> | ||
| <h5 className='cio-section-name'>{camelToStartCase(sectionTitle)}</h5> | ||
| <div className='cio-section-items'> | ||
| {section?.data?.map((item) => renderItem(item))} | ||
| </div> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way we can get this from getSectionProps since this same logic is being duplicated
https://github.com/Constructor-io/constructorio-ui-autocomplete/pull/128/files#diff-7f1c987b478566cc9cf82a2afc2ee7a7a150ec6047a7801e0ac01c410f7ce453R218
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could. The reason why its included here is because we've rendered a separate
divwith the relevant classes in Line 154 so we kind of have to duplicate the logic to get the required values here.Took a look at our history and it might be something we've included prior to creating
getSectionPropsso we might be good to omit this entirely from the story. @esezen would love to get your thoughts here since you touched it previously.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In terms of passing this variable via
getSectionPropsitself, I thoughtgetSectionPropsis meant to contain only the attributes we're going to render into the HTML? I would be against adding something in specifically for our Storybook.