style(list-item): add connotation support for activated list items

[JoelGraham93]

@jshenkman the text in dark mode for connotation=cta is black in this PR.
The figma file had some white text examples but I thought they might need updating?
https://www.figma.com/file/r8OYLPmRmDqfLKt6KHdJDB/Vonage-UI-Kit---2.0?node-id=4758%3A2725

[github-actions]

🚀

Latest successful build of the PR deployed here.

🚀

[jshenkman]

@jshenkman the text in dark mode for connotation=cta is black in this PR.
The figma file had some white text examples but I thought they might need updating?
https://www.figma.com/file/r8OYLPmRmDqfLKt6KHdJDB/Vonage-UI-Kit---2.0?node-id=4758%3A2725

Yes, you are right! thanks!
updated the design, so for the light theme the text is white for both primary and CTA, and for the dark theme it's black text for both connotations

[yinonov]

how complex would it be to proxy contextual variants (like shape, connotation) to list itself rather than list item?

[JoelGraham93]

how complex would it be to proxy contextual variants (like shape, connotation) to list itself rather than list item?

We could use a similar approach to the accordion openAll prop that we decided against using:

vivid/components/expansion-panel/src/vwc-expansion-panel-list-base.ts

Lines 18 to 27 in 8b0d1be

	connectedCallback(): void {
	super.connectedCallback();
	this.expansionPanels = this.children as HTMLCollectionOf<VWCExpansionPanelBase>;
	if (this.multi && this.openAll && this.expansionPanels) {
	for (const expansionPanel of this.expansionPanels) {
	expansionPanel.open = true;
	}
	}
	}

[yinonov]

how complex would it be to proxy contextual variants (like shape, connotation) to list itself rather than list item?

We could use a similar approach to the accordion openAll prop that we decided against using:

vivid/components/expansion-panel/src/vwc-expansion-panel-list-base.ts

Lines 18 to 27 in 8b0d1be

	connectedCallback(): void {
	super.connectedCallback();
	this.expansionPanels = this.children as HTMLCollectionOf<VWCExpansionPanelBase>;
	if (this.multi && this.openAll && this.expansionPanels) {
	for (const expansionPanel of this.expansionPanels) {
	expansionPanel.open = true;
	}
	}
	}

how about setting the css values from the list styles. this will prevent any mutations to list items. we practice it in toggle buttons and here

[yinonov]

connotated list items have no ripple indication

[yinonov]

how complex would it be to proxy contextual variants (like shape, connotation) to list itself rather than list item?

We could use a similar approach to the accordion openAll prop that we decided against using:

vivid/components/expansion-panel/src/vwc-expansion-panel-list-base.ts

Lines 18 to 27 in 8b0d1be

	connectedCallback(): void {
	super.connectedCallback();
	this.expansionPanels = this.children as HTMLCollectionOf<VWCExpansionPanelBase>;
	if (this.multi && this.openAll && this.expansionPanels) {
	for (const expansionPanel of this.expansionPanels) {
	expansionPanel.open = true;
	}
	}
	}

how about setting the css values from the list styles. this will prevent any mutations to list items. we practice it in toggle buttons and here

@JoelGraham93 where does this stand?

[JoelGraham93]

@JoelGraham93 where does this stand?

I like this, moving forward with css approach. Just updated the shape implementation

[yinonov]

@JoelGraham93 where does this stand?

I like this, moving forward with css approach. Just updated the shape implementation

@JoelGraham93 my recommended solution was either to set styles by defining their css variables from the container OR use our API to mutate the child nodes attributes (which might cost in performance in the case of many list items).
We currently practice both approaches in various integrations to apply API to a collective.
in the case here, you defined style props directly and not css variables. this may solve the shape style but isn’t scalable or enables a practice of other APIs (e.g. connotation) that depend on shadowRoot styling and css variables.
If we refactor this and apply desired style by setting attributes to list items we may run into performance issues as mentioned above.
therefore I suggest reverting the changes of shape to only be set by list items alone (as it was before).
Alternately, we have some interesting thoughts on theming coming up… to enable this in a more declarative manner from within the html

[JoelGraham93]

@JoelGraham93 where does this stand?

I like this, moving forward with css approach. Just updated the shape implementation

@JoelGraham93 my recommended solution was either to set styles by defining their css variables from the container OR use our API to mutate the child nodes attributes (which might cost in performance in the case of many list items).
We currently practice both approaches in various integrations to apply API to a collective.
in the case here, you defined style props directly and not css variables. this may solve the shape style but isn’t scalable or enables a practice of other APIs (e.g. connotation) that depend on shadowRoot styling and css variables.
If we refactor this and apply desired style by setting attributes to list items we may run into performance issues as mentioned above.
therefore I suggest reverting the changes of shape to only be set by list items alone (as it was before).
Alternately, we have some interesting thoughts on theming coming up… to enable this in a more declarative manner from within the html

Ok I've reverted and will see how the theming progresses.
I'm not a big fan of encouraging consumers to use css vars unless it's a unique case - with miss-use it could make any rebranding difficult in the future

[yinonov]

@JoelGraham93 where does this stand?

I like this, moving forward with css approach. Just updated the shape implementation

@JoelGraham93 my recommended solution was either to set styles by defining their css variables from the container OR use our API to mutate the child nodes attributes (which might cost in performance in the case of many list items).
We currently practice both approaches in various integrations to apply API to a collective.
in the case here, you defined style props directly and not css variables. this may solve the shape style but isn’t scalable or enables a practice of other APIs (e.g. connotation) that depend on shadowRoot styling and css variables.
If we refactor this and apply desired style by setting attributes to list items we may run into performance issues as mentioned above.
therefore I suggest reverting the changes of shape to only be set by list items alone (as it was before).
Alternately, we have some interesting thoughts on theming coming up… to enable this in a more declarative manner from within the html

Ok I've reverted and will see how the theming progresses.
I'm not a big fan of encouraging consumers to use css vars unless it's a unique case - with miss-use it could make any rebranding difficult in the future

Thanks. it was never the intention for consumers to do such overrides. see discussion

[yinonov]

still relevant? should move back to list-item

[yinonov]

still relevant? should move back to list-item

[JoelGraham93]

I reverted to a commit to remove all this let me check

[JoelGraham93]

@yinonov all is correctly reverted now

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication