Widget CSS conflicts with my app #126

Closed
jfairley opened this Issue Oct 17, 2016 · 23 comments

Comments

Projects
None yet
@jfairley

I like that Okta provides this sign-in widget to help me get going, but the use of reset styles and other generic styles makes the provided CSS unusable

https://github.com/okta/okta-signin-widget/blob/okta-signin-widget-1.7.0/assets/sass/common/enduser/_reset.scss
https://github.com/okta/okta-signin-widget/blob/okta-signin-widget-1.7.0/assets/sass/widgets/_mega-drop-down.scss

Specifically, the global input styles and the generic .dropdown styles conflict with styles already built in my application, forcing me to completely style the Okta widget on my own. Something like this always needs to be scoped under an Okta class so as to not collide with everything else in your clients' custom apps.

@rchild-okta

This comment has been minimized.

Show comment
Hide comment
@rchild-okta

rchild-okta Oct 18, 2016

Member

Definitely makes sense - it's something we can go ahead and fix in an upcoming release.

Member

rchild-okta commented Oct 18, 2016

Definitely makes sense - it's something we can go ahead and fix in an upcoming release.

alanzhou-okta added a commit to alanzhou-okta/okta-signin-widget that referenced this issue Oct 31, 2016

🌱 Adds close, hide, and show functions for widget
Exposes functions to close, hide, and show widget

Resolves #126

alanzhou-okta added a commit to alanzhou-okta/okta-signin-widget that referenced this issue Oct 31, 2016

🐛 Removes okta css from affecting single-app css
Adds a ScssContainer so css only affects that container
Moves scss imports so they only affect the ScssContainer

Resolves #126

alanzhou-okta added a commit to alanzhou-okta/okta-signin-widget that referenced this issue Oct 31, 2016

🐛 Removes okta css from affecting single-app css
Adds a ScssContainer so css only affects that container
Moves scss imports so they only affect the ScssContainer

Resolves #126

alanzhou-okta added a commit to alanzhou-okta/okta-signin-widget that referenced this issue Oct 31, 2016

🐛 Removes okta css from affecting single-app css
Adds a ScssContainer so css only affects that container
Moves scss imports so they only affect the ScssContainer

Resolves #126

alanzhou-okta added a commit to alanzhou-okta/okta-signin-widget that referenced this issue Nov 1, 2016

🌱 Adds close, hide, and show functions for widget
Exposes functions to close, hide, and show widget

Resolves #126

alanzhou-okta added a commit to alanzhou-okta/okta-signin-widget that referenced this issue Nov 1, 2016

🐛 Removes okta css from affecting single-app css
Adds a ScssContainer so css only affects that container
Moves scss imports so they only affect the ScssContainer

Resolves #126

alanzhou-okta added a commit to alanzhou-okta/okta-signin-widget that referenced this issue Nov 2, 2016

🐛 Removes okta css form affecting single-app css
Wraps majority of css inside okta container so it doesn't affect other css

Resolves #126

alanzhou-okta added a commit to alanzhou-okta/okta-signin-widget that referenced this issue Nov 2, 2016

🐛 Removes okta css form affecting single-app css
Wraps majority of css inside okta container so it doesn't affect other css

Resolves #126

alanzhou-okta added a commit to alanzhou-okta/okta-signin-widget that referenced this issue Nov 3, 2016

🐛 Removes okta css form affecting single-app css
Wraps majority of css inside okta container so it doesn't affect other css

Resolves #126

alanzhou-okta added a commit to alanzhou-okta/okta-signin-widget that referenced this issue Nov 4, 2016

🐛 Removes okta css form affecting single-app css
Wraps majority of css inside okta container so it doesn't affect other css

Resolves #126
@lawrencetvo

This comment has been minimized.

Show comment
Hide comment
@lawrencetvo

lawrencetvo Mar 13, 2017

+1 important to one of my related projects.

+1 important to one of my related projects.

@mraible

This comment has been minimized.

Show comment
Hide comment
@mraible

mraible Mar 14, 2017

Member

I agree it'd be nice to have this fixed. Here's a new Angular app without the Sign In widget's CSS:

screen shot 2017-03-14 at 1 07 27 pm

When I add the CSS, the <h1> that says "app works!" becomes much smaller and all the margins/padding go away.

screen shot 2017-03-14 at 1 07 42 pm

Member

mraible commented Mar 14, 2017

I agree it'd be nice to have this fixed. Here's a new Angular app without the Sign In widget's CSS:

screen shot 2017-03-14 at 1 07 27 pm

When I add the CSS, the <h1> that says "app works!" becomes much smaller and all the margins/padding go away.

screen shot 2017-03-14 at 1 07 42 pm

@sasso

This comment has been minimized.

Show comment
Hide comment
@sasso

sasso Mar 23, 2017

Quick fix is to comment out

//@import 'common/enduser/reset';
//@import 'base';

and build a release

sasso commented Mar 23, 2017

Quick fix is to comment out

//@import 'common/enduser/reset';
//@import 'base';

and build a release

@omgitstom

This comment has been minimized.

Show comment
Hide comment
@omgitstom

omgitstom May 25, 2017

Member

This is a bug, not an enhancement

Member

omgitstom commented May 25, 2017

This is a bug, not an enhancement

@pith

This comment has been minimized.

Show comment
Hide comment
@pith

pith Jun 2, 2017

Any plan on releasing this fix ? This widget is awesome but this bug makes it really annoying.

pith commented Jun 2, 2017

Any plan on releasing this fix ? This widget is awesome but this bug makes it really annoying.

@AdrianZielonka

This comment has been minimized.

Show comment
Hide comment
@AdrianZielonka

AdrianZielonka Jun 7, 2017

+1. Any word on when this fix will be released? Our organisation has just purchased Okta and would like to utilise the Sign In Widget, however it hijacks our entire SPAs CSS.

AdrianZielonka commented Jun 7, 2017

+1. Any word on when this fix will be released? Our organisation has just purchased Okta and would like to utilise the Sign In Widget, however it hijacks our entire SPAs CSS.

@omgitstom

This comment has been minimized.

Show comment
Hide comment
@omgitstom

omgitstom Jun 13, 2017

Member

We are going to start work on getting a release out for this issue, thanks for your patience!

Member

omgitstom commented Jun 13, 2017

We are going to start work on getting a release out for this issue, thanks for your patience!

@ericmcgregor

This comment has been minimized.

Show comment
Hide comment
@ericmcgregor

ericmcgregor Jun 21, 2017

I also am working on integrating Okta with some applications at my company and this is a bit of a blocker.

I also am working on integrating Okta with some applications at my company and this is a bit of a blocker.

@StateBarofArizona

This comment has been minimized.

Show comment
Hide comment
@StateBarofArizona

StateBarofArizona Jun 27, 2017

Agreed that this is a HUGE bug in the widget. The okta-sign-in.min.css trashes the CSS in my site no matter where I load it.

Agreed that this is a HUGE bug in the widget. The okta-sign-in.min.css trashes the CSS in my site no matter where I load it.

@sasso

This comment has been minimized.

Show comment
Hide comment

sasso commented Jun 27, 2017

@omgitstom any updates?

@mraible

This comment has been minimized.

Show comment
Hide comment
@mraible

mraible Jun 27, 2017

Member

@sasso I tested a branch of the widget today that has this fixed. Not sure when it will be released, but hopefully soon.

Member

mraible commented Jun 27, 2017

@sasso I tested a branch of the widget today that has this fixed. Not sure when it will be released, but hopefully soon.

@sasso

This comment has been minimized.

Show comment
Hide comment
@sasso

sasso Jul 1, 2017

It's pretty disappointing that Okta is even working on other features and bugfixes as this is a complete blocker for anyone using this widget... 10 months from when this was first reported and still no fix (it's only css) 😔

sasso commented Jul 1, 2017

It's pretty disappointing that Okta is even working on other features and bugfixes as this is a complete blocker for anyone using this widget... 10 months from when this was first reported and still no fix (it's only css) 😔

@therealnicksaunders

This comment has been minimized.

Show comment
Hide comment
@therealnicksaunders

therealnicksaunders Jul 17, 2017

I would suggest scope-css as a workaround for anyone who's blocked. (Personally, I created a Webpack loader to roll it into my build process.)

therealnicksaunders commented Jul 17, 2017

I would suggest scope-css as a workaround for anyone who's blocked. (Personally, I created a Webpack loader to roll it into my build process.)

@lboyette-okta

This comment has been minimized.

Show comment
Hide comment
@lboyette-okta

lboyette-okta Jul 19, 2017

Contributor

@sasso @mraible The fixed version (2.1.0) has been published to npm.

Contributor

lboyette-okta commented Jul 19, 2017

@sasso @mraible The fixed version (2.1.0) has been published to npm.

@mraible

This comment has been minimized.

Show comment
Hide comment
@StateBarofArizona

This comment has been minimized.

Show comment
Hide comment
@StateBarofArizona

StateBarofArizona Aug 4, 2017

What is the status on the fix to 2.1.0 to correct the missing checkbox images?

What is the status on the fix to 2.1.0 to correct the missing checkbox images?

@mraible

This comment has been minimized.

Show comment
Hide comment
@mraible

mraible Aug 7, 2017

Member

@StateBarofArizona I've created a PR to fix this at #248. Trying to push it through this week.

Member

mraible commented Aug 7, 2017

@StateBarofArizona I've created a PR to fix this at #248. Trying to push it through this week.

@therealnicksaunders

This comment has been minimized.

Show comment
Hide comment
@therealnicksaunders

therealnicksaunders Aug 11, 2017

At the moment, I am using the sign-in widget @ ^1.13.0. Sadly the CSS issues have prevented me from directly including the stylesheet in my HTML document; instead, I have to pipe it through my build process which scopes the selectors to the element in which the widget is rendered and also corrects the checkbox image paths apparently fixed in the as-of-yet-unmerged PR #248.

I attempted to upgrade to 2.1.0 and of course the checkbox images are still broken. I am going to second @StateBarofArizona and suggest that @mraible's work really needs to be merged ASAP because I am sure you don't want clients having to implement this patch in a build process like I am doing.

Furthermore, I am going to ask @lboyette-okta to take another look at the selector scoping implementation. It looks like you guys have assigned an ID to the widget element and are using ID selectors, but this gives your CSS a high degree of specificity that still clobbers any styles that we try to apply to elements within the sign-in widget. Would you please apply a class like .okta-signin-widget instead of an ID like #okta-signin-widget.

I have had two different colleagues ask me about the wacky-looking build steps that I have added to resolve these issues, and, frankly, it's embarrassing. I can't believe these would take more than a day to resolve fully, let alone 10 months!

At the moment, I am using the sign-in widget @ ^1.13.0. Sadly the CSS issues have prevented me from directly including the stylesheet in my HTML document; instead, I have to pipe it through my build process which scopes the selectors to the element in which the widget is rendered and also corrects the checkbox image paths apparently fixed in the as-of-yet-unmerged PR #248.

I attempted to upgrade to 2.1.0 and of course the checkbox images are still broken. I am going to second @StateBarofArizona and suggest that @mraible's work really needs to be merged ASAP because I am sure you don't want clients having to implement this patch in a build process like I am doing.

Furthermore, I am going to ask @lboyette-okta to take another look at the selector scoping implementation. It looks like you guys have assigned an ID to the widget element and are using ID selectors, but this gives your CSS a high degree of specificity that still clobbers any styles that we try to apply to elements within the sign-in widget. Would you please apply a class like .okta-signin-widget instead of an ID like #okta-signin-widget.

I have had two different colleagues ask me about the wacky-looking build steps that I have added to resolve these issues, and, frankly, it's embarrassing. I can't believe these would take more than a day to resolve fully, let alone 10 months!

@santhoshbalakrishnan

This comment has been minimized.

Show comment
Hide comment
Contributor

santhoshbalakrishnan commented Aug 19, 2017

@therealnicksaunders @StateBarofArizona @mraible ℹ️ The checkbox fix is in with #284

@rchild-okta

This comment has been minimized.

Show comment
Hide comment
@rchild-okta

rchild-okta Sep 12, 2017

Member

Closing - fix for the main issue is in 2.1.0, and #284 is in master (will be in the next release).

Member

rchild-okta commented Sep 12, 2017

Closing - fix for the main issue is in 2.1.0, and #284 is in master (will be in the next release).

@therealnicksaunders

This comment has been minimized.

Show comment
Hide comment
@therealnicksaunders

therealnicksaunders Sep 12, 2017

@rchild-okta I would like to reiterate that using ID selectors as a fix for this issue is problematic because it makes customizing the appearance of the sign-in widget difficult by creating a high degree of specificity in the selectors in the default stylesheet. In other words, our designer has difficulty overriding the default styles because the browser thinks the defaults are more important than his customizations. Is there any chance you guys can simply use a class instead of an ID?

@rchild-okta I would like to reiterate that using ID selectors as a fix for this issue is problematic because it makes customizing the appearance of the sign-in widget difficult by creating a high degree of specificity in the selectors in the default stylesheet. In other words, our designer has difficulty overriding the default styles because the browser thinks the defaults are more important than his customizations. Is there any chance you guys can simply use a class instead of an ID?

@rchild-okta

This comment has been minimized.

Show comment
Hide comment
@rchild-okta

rchild-okta Sep 12, 2017

Member

@therealnicksaunders mind opening another github issue for that? It's something we're looking to fix, but IIRC is a bit involved - we use an internal library for most of our components, and needed that extra specificity to workaround that implementation. We've been cleaning it up internally (it might already be at a point where we can swap it out), but requires some investigation.

Member

rchild-okta commented Sep 12, 2017

@therealnicksaunders mind opening another github issue for that? It's something we're looking to fix, but IIRC is a bit involved - we use an internal library for most of our components, and needed that extra specificity to workaround that implementation. We've been cleaning it up internally (it might already be at a point where we can swap it out), but requires some investigation.

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