-
Notifications
You must be signed in to change notification settings - Fork 4
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
QGDS-22-Accordion Init #247
base: develop
Are you sure you want to change the base?
Conversation
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.
hbs cleanup is interesting but we do need to think about how QH and many others in handling the migration to the new template. i.e. migration guide required.
Also how to include this in a mega json array that has a simple template that auto loads any objects dynamically. see https://66c421d9ea5f69287ba33241-zarbnywjry.chromatic.com/?path=/story/1-styles-typography--component-global-elements which is run off https://github.com/qld-gov-au/qgds-vanilla/blob/develop/src/stories/templates/global-body/global-body-all_typography.json
{{{intro}}} | ||
{{/if}} | ||
|
||
<div class="qld__accordion-group qld__accordion-group--{{theme}}" id="accordion-group--{{id}}"> |
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 need to move away from qld__accordion-group--{{theme}} to just using the 'qld-{{theme}}' to css bloat.
</div> | ||
{{/if}} | ||
<ul class="qld__accordion-list"> | ||
{{#each component}} |
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.
you have dropped {{#printAccordion metadata}}
did you work out what that was?
import mockupData from "./accordion.data.json"; | ||
|
||
export default { | ||
title: "Components / Accordion", |
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.
was title: "Components/Accordion",
title: "Components / Accordion", | ||
render: (args) => { | ||
try { | ||
return new QGDS.Accordion({ data: args }).htmlstring; |
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.
this is over engineered, just do
render: ( args) => {
handlebarsInit(Handlebars)
try {
return Handlebars.compile("{{> accordion }}")(args)
} catch (e) {
console.log(e)
return JSON.stringify(e) + JSON.stringify(args);
}
},
options: ["light", "alt", "dark", "dark-alt"], | ||
}, | ||
}, | ||
}; |
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.
please put back the figma links
/**
* Additional parameters for the story.
*
* @type {Object}
* @property {Object} design - Configuration for the design parameter.
* @property {string} design.name - Name of the design parameter.
* @property {string} design.type - Type of the design parameter.
* @property {string} design.url - URL of the design parameter.
*/
parameters: {
design: {
name: "QGDS Figma Reference",
type: "figma",
url: "https://www.figma.com/design/qKsxl3ogIlBp7dafgxXuCA/QLD-GOV-DDS?node-id=23167-395554",
},
},
theme: "dark-alt", | ||
id: "131", | ||
}, | ||
}; |
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.
please add back in the example html versions for css diffing.
/**
* Default Accordion story
*/
export const Default = {};
/**
* Single light
*/
export const SingleLight = {
render: () => {
return example2html;
},
args: {},
globals: { theme: 'None'},
}
/**
* Single dark
*/
export const SingleDark = {
render: () => {
return example3html;
},
args: {},
globals: { theme: 'Dark'},
}
/**
* Multi light
*/
export const MultiLight = {
render: () => {
return example4html;
},
args: {},
globals: { theme: 'None'},
}
/**
* Single dark
*/
export const MultiDark = {
render: () => {
return example5html;
},
args: {},
globals: { theme: 'Dark' },
}
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.
i thought this would have broken https://github.com/qld-gov-au/qgds-vanilla/blob/develop/src/components/_global/css/body/stories/allTypography.json but its not used there.
Be aware that the move to removing the .data. wrapper does make it harder to do have a global template and just pass in a huge json file. like
https://github.com/qld-gov-au/qgds-vanilla/blob/develop/src/stories/templates/global-body/global-body-all_typography.json
e.g.
{
"items": [
{
"component": {
"type": "h1",
"data": {
"metadata": {
"id_field": {
"value": "h1id"
}
},
"content": {
"value": "Heading 1"
}
}
}
},
now cant be auto including this template as its not conformant to the pattern used everywhere else.
unsure how we should go forward on this. i.e. have this called internal which is more streamlined and have the accordion hbs re jig the array to pass through, or a js helper function added to do the rejig pass through to stay backwards compatible.
@@ -0,0 +1,8 @@ | |||
/** |
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 this for building or for browser runtime?
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.
please not delete, this is developer docs on accordion.
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.
please not delete and look at streamlining to just use qld--alt, qld--dark, qld--alt, instead of the double wrap.
Init: Accordion file restructured.
note: Will need to look into the global.js file, as there are a few strange things happening with multiple accordions on the page.