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

Scoped CSS capabilities per-component #75

Closed
seangates opened this issue Apr 3, 2018 · 18 comments
Closed

Scoped CSS capabilities per-component #75

seangates opened this issue Apr 3, 2018 · 18 comments

Comments

@seangates
Copy link
Contributor

The current Skin scoped CSS does not provide scoping for individual components which are used from the eBayUI Core library. It only provides scoping when Skin is used in isolation (no eBayUI components).

Therefore, we need to provide a way to scope the individual components when they are used within a project.

Some ideas:

  1. Allow a specifically scoped .ds6 version of the Skin css within each component
  2. Create a special scoped class on the main container when a certain flag is encountered
  3. Use ARC to provide a wholly separate component path
  4. Copy the corresponding Skin module CSS into a local LESS file for each component

Please let me know if any of you have questions about the reasons, and please chime in if you have an idea about possible solutions.

@senthilp
Copy link

senthilp commented Apr 3, 2018

@seangates I am not sure if this is needed for core components? If scoping is needed, application teams can create their own wrapper. Core components are the default mechanisms, where we reserve the namespace.

@seangates
Copy link
Contributor Author

seangates commented Apr 3, 2018

@senthilp The issue is when someone uses a component, there is a peer dependency for Skin, which is un-scoped. Therefore, we still get the un-scoped CSS bundled.

Mostly, we just need a better solve for the scoping issue for the Global Header team. What we currently have provided them through Skin is not sufficient.

@senthilp
Copy link

senthilp commented Apr 3, 2018

@seangates For core components, this is expected. Skin classes should come as un-scoped. The Global Header is a special case and also a temporary one. My only concern is that we should not be spending the time to come up with an elegant solution for Global Header, instead, have the GH team put some overrides on their end.

@seangates
Copy link
Contributor Author

Sure, but at the same time I would like to reduce bloat, which could be a real concern.

@DylanPiercey
Copy link
Member

DylanPiercey commented Apr 3, 2018

So I think this issue may not be isolated to global header and might warrant some more discussion. Currently Anton from myebay c2c selling is running into a similar issue because their page is a SPA and both the skin ds6 and ds4 styles are loaded while you are navigating on the page.

@senthilp
Copy link

senthilp commented Apr 3, 2018

I also discussed this with Anton and the proper solution for his scenario is to upgrade his SPA to DS6 and always pass the ds6 flag. Do they have a problem even if they upgrade fully to Skin 3.x for all their SPA pages?

@DylanPiercey
Copy link
Member

No, they would not have an issue if they upgraded all pages.

@RajaRamu
Copy link
Contributor

RajaRamu commented Apr 4, 2018

@senthilp - Based the initial discussion, any page can work with ds6 and ds4 without conflicting each other to gracefully migrate to ds6 and ebayui components. if thats true, then we may have to consider this to address it. As of now, we tend to copy the ebayui components to Global header repo which is not good and scalable

@seangates
Copy link
Contributor Author

@RajaRamu What do you mean "scalable"? For the GH team? For eBayUI?

@CestDiego
Copy link
Contributor

My 2 cents:

We currently have a use case in which we inject modules into any page (ds4, ds6, or no skin at all) through AJAX. We also are using ds6 in our modules.

The first problem we ran into was that with our own GH developed modules, when including skin, we were affecting the host page's style (grids, buttons, etc styles would change). This is where the scoped skin solution came in handy.

When we require modules that also bundle skin by themselves, such as the ebayUI core components, we are also bundling the unscoped skin version too. This not only causes bloating (two skin versions being flushed down to the browser via AJAX) but also takes us back pre-scoped-skin era, in which we are affecting the host page styles.

I have a proposed solution that has not been mentioned and I hope it's not too hairy:
I've noticed that core components have a 1:1 ratio with skin components. i.e. dialog, button, etc. That means that they do not need custom CSS by themselves. If GH as a user of these modules could pass a lasso flag, so that ebayUI doesn't bundle skin at all for any component, then we become responsible to add there our own scoped skin bundle.

This might work, but I don't really see how this could be useful to anyone else. It is almost anti-pattern-y.

I'm happy to continue discussions of other proposals as well

@seangates
Copy link
Contributor Author

So, something like a no-style lasso flag?

@CestDiego
Copy link
Contributor

CestDiego commented Apr 4, 2018 via email

@RajaRamu
Copy link
Contributor

RajaRamu commented Apr 4, 2018

@seangates - Not scalable* - For every change on ebay-*components, we will have to copy it over. this may lead to feature disparity as feature grows.

@seangates
Copy link
Contributor Author

Sorry, that still doesn't make sense, @RajaRamu.

Are you trying to say: if you folks make overrides, and we update Skin, you'll have to match the code written in GH styles?

@mlrawlings
Copy link

mlrawlings commented Apr 4, 2018

So I think this issue may not be isolated to global header and might warrant some more discussion. Currently Anton from myebay c2c selling is running into a similar issue because their page is a SPA and both the skin ds6 and ds4 styles are loaded while you are navigating on the page.

Just to update, the MyEbay application now has these components copied into their project because they need to remove the skin css and replace it with a scoped version to support multiple pages with different design systems. A no-styles flag or similar would work for their use-case.

@senthilp
Copy link

senthilp commented Apr 4, 2018

@mlrawlings That is the wrong way of doing it. This is just unnecessary tech debt. Let me have a chat with MyeBay team and see what we can do.
@RajaRamu again the use case you mentioned still applies to GH only. If a page is doing an AJAX call, they still can control what they need or not because they own the page.

Having said that, I still like the solution proposed by @CestDiego I would go with a skinless flag for eBayUI to ignore Skin modules as a part of the bundling process.

@RajaRamu
Copy link
Contributor

RajaRamu commented Apr 6, 2018

Per discussion -
• skin scoping, We are all in the same page of the impact that no page can work with ds4 and ds6 together. It will be either ds4 or ds6. If any page wants to use ebayui components, entire page should be using latest skin which ebayui-core uses
• ebayui-core will honor the skin-less lasso flag to strip off the skin from ebayui-core and it’s up to GH team to put the scoped skin in the application level. This is the blocker to use ebayui-components which will be prioritized in the upcoming sprint

@ianmcburnie
Copy link
Contributor

Done in 0.2.0 release.

v0.2.x automation moved this from Pull request to Done Apr 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
v0.2.x
  
Done
Development

No branches or pull requests

8 participants