Skip to content
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

[Accordion] Its .content and .title classes shouldn’t override the corresponding classes of nested components. #4928

Open
dmaison opened this issue Jan 20, 2017 · 9 comments

Comments

@dmaison
Copy link

dmaison commented Jan 20, 2017

Steps to Reproduce

  1. Create accordion
  2. Add header inside accordion

Expected
The header should be formatted correctly

Result
The inherent accordion style interferes with the formatting of the header

Testcase
http://jsfiddle.net/p9L4c6Lh/1/

@Darkesthour
Copy link

Hello.
But why do you use <div class="content"/> inside <h2 class="ui dividing header"/>? It works perfectly without this extra-container (it's because content class has a special purpose inside <div class="ui accordion"/>).

@BitForger
Copy link

to sum up @Darkesthour explanation.. remove the div.content sitting inside the dividing header.

@dmaison
Copy link
Author

dmaison commented Jan 23, 2017

@Cyb3rWarri0r8 @Darkesthour, this solution doesn't account for adding sub headers, or even any other type of semantic style group that can utilize a content class.

The way I see it, specific styles shouldn't conflict with other styles in semantic. Changing the style for accordion content from .ui.accordion .content to .ui.accordion > .content would prevent any foreseeable conflicting issues.

@BitForger
Copy link

@dmaison Well there is the ability to override that if you want to... I haven't tried this, but for things that say to have a div.content have you tried just using it without the content wrapper?

@awgv awgv changed the title [Accordion] - headers inside accordions [Accordion] Its “.content” class shouldn’t override a “.content” class of nested components. Feb 4, 2017
@Unsfer
Copy link

Unsfer commented Apr 5, 2017

One more appropriate testcase, this time with Floated List:
http://jsfiddle.net/fseb296h/

And in my case I can't just remove "content" class, because I'm using Semantic-UI-React.
I also think, that .ui.accordion > .content will be just fine

@awgv awgv added this to the Needs Milestone milestone Apr 5, 2017
@awgv awgv changed the title [Accordion] Its “.content” class shouldn’t override a “.content” class of nested components. [Accordion] Its .content and .title classes shouldn’t override the corresponding classes of nested components. May 3, 2017
@awgv
Copy link
Member

awgv commented May 3, 2017

The search component seems to be affected as well: #5330.

@stale
Copy link

stale bot commented Feb 23, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 30 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Feb 23, 2018
@stale stale bot closed this as completed Mar 25, 2018
@awgv awgv reopened this Mar 25, 2018
@stale stale bot removed the stale label Mar 25, 2018
@attaboyer-dev
Copy link

attaboyer-dev commented Nov 5, 2018

I've recently encountered this by using the <Message.List /> component within a stylized accordion in Semantic-Ui-React.

The suggested .ui.accordion > .content referenced by @dmaison was able to resolve my issue, however, it still seems inappropriate for the accordion content to clash with other components as a default behavior.

@lubber-de

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants