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

Disable built in component styles #497

Closed
knoobie opened this issue Aug 10, 2019 · 5 comments · Fixed by #526
Closed

Disable built in component styles #497

knoobie opened this issue Aug 10, 2019 · 5 comments · Fixed by #526

Comments

@knoobie
Copy link
Contributor

knoobie commented Aug 10, 2019

After upgrading our application to 4.6 (from 3.x) - I noticed that you have changed your styling approach from css files to js included css. Appreciate that!

It just comes with the little downside, when you explicit wanna exclude some css. We removed all the button styling before - so your default styling won't interfere with bootstrap's styling.

Example:

buttons: [
    {
      action() {
        return this.cancel();
      },
      classes: 'btn btn-secondary',
      text: 'Beenden'
    }, {
      action() {
        return this.next();
      },
      classes: 'btn btn-primary',
      text: 'Weiter'
    }
  ]

Now your classes overwrite the bootstrap styling because of your inline css. Is there a way to disable for example the css of the buttons? Like "removeClasses" or "disableStyling".

Proposal (sorry, no js guy)

grafik

@RobbieTheWagner
Copy link
Member

@knoobie wouldn't the override approach still be the same? You can pass custom classes and use custom CSS to override, as needed. You might need !important occasionally.

@knoobie
Copy link
Contributor Author

knoobie commented Aug 12, 2019

@rwwagner90 the problem with custom classes or let's say !important is, that for example bootstrap doesn't uses important. So we are kinda stuck ;)

The difference would be that your classes aren't added anymore. Only the self-defined custom classes

@RobbieTheWagner
Copy link
Member

@knoobie You would have to add style overrides, just like before. I'm not sure I see the problem. You can add the classes and add CSS for them. You could also add your own style tags like we do, and override without !important.

I'm not against a way to exclude all styles, as several people have requested it, but in my opinion, Shepherd is pretty useless without any styles, so how to include just the right amount of styles needs to be well thought out before we do anything.

@knoobie
Copy link
Contributor Author

knoobie commented Aug 12, 2019

Shepherd is pretty useless without any styles

I think you are right with that! The hardest part is - what has to be included, what can be optional.

Well let me say it like this, currently the code above produces:

<button class="btn btn-primary shepherd-button">
vs
<button class="btn btn-primary">

This way no css has to be overwritten, because nothing could be applied with the selector . shepherd-button. Drawback: those 20-30 lines of css are still distributes (okay in my opinion in the first draft)

@RobbieTheWagner
Copy link
Member

This needs much more planning before we can implement a solution, unfortunately. I would recommend overriding for now.

@RobbieTheWagner RobbieTheWagner changed the title Feature Request: Disable build in style of components Disable build in style of components Aug 21, 2019
@RobbieTheWagner RobbieTheWagner changed the title Disable build in style of components Disable built in style of components Aug 21, 2019
@RobbieTheWagner RobbieTheWagner changed the title Disable built in style of components Disable built in component styles Aug 21, 2019
RobbieTheWagner added a commit that referenced this issue Aug 22, 2019
This will allow users who find themselves overriding a lot of styles to opt out of our base styles. Closes #497
RobbieTheWagner added a commit that referenced this issue Aug 22, 2019
This will allow users who find themselves overriding a lot of styles to opt out of our base styles. Closes #497
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants