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

Support configuring top level element #1199

Closed
tfavorite opened this issue Nov 9, 2018 · 17 comments
Closed

Support configuring top level element #1199

tfavorite opened this issue Nov 9, 2018 · 17 comments
Assignees
Labels
focus: refactor Label for tasks we should include for a major release refactor status: on hold ⏸️ type: breaking change 💥 type: enhancement ✨ [5] Velocity rating (Fibonacci)

Comments

@tfavorite
Copy link
Contributor

tfavorite commented Nov 9, 2018

Is your feature request related to a problem? Please describe.
Our usage of IDS is limited to a section of the page, not the full body. Therefore in order to use it we have resorted to forking many of the components that make this assumption, replacing references to the body selector with a selector that we have specified.
For example, in the tabs component, we've replaced this line:
menuHtml = $(``<ul id="tab-container-popupmenu" class="tab-list-spillover${shouldBeSelectable}">``).appendTo('body');
with this:
menuHtml = $(``<ul id="tab-container-popupmenu" class="tab-list-spillover${shouldBeSelectable}">``).appendTo(window.$FEF_TOP_LEVEL_ELEMENT_SELECTOR);

Describe the solution you'd like
A way to configure the top level element where IDS/Soho will be referenced.

Describe alternatives you've considered
Other than continuing to fork, we don't have any. Also another alternative is to prefix all components for example ids-grid, ids-dropdown ect. This may be more work but might be more flexible

Additional context
This is a request from the GT Nexus Front-End Framework team.

@tmcconechy tmcconechy added type: enhancement ✨ [∞] Velocity rating (Fibonacci) labels Nov 9, 2018
@tmcconechy tmcconechy added [5] Velocity rating (Fibonacci) and removed [∞] Velocity rating (Fibonacci) labels Oct 4, 2019
@tmcconechy
Copy link
Member

I think we do need this for other cases so the trick would be to just add an ids-enterprise top level class and let it cascade only after that. Would be a breaking change we can mitigate easily. Could also add it to initialize() when you call that (if people use it).

@Fruko
Copy link

Fruko commented Oct 7, 2019

Would it be possible to add support for soho to add support for ignoring elements?
In our use-case we want app styled according to Soho, but we need a way to tell Soho css to ignore certain elements and it's descendants

e.g. elements with class would be ignored

.soho-ignore

perhaps :not() selector would be of help?

@EdwardCoyle EdwardCoyle added this to To do in Enterprise 4.23.x (October 2019) Sprint via automation Oct 7, 2019
@tmcconechy
Copy link
Member

tmcconechy commented Oct 7, 2019

I'm not sure that would be easy @Fruko I guess we would have to add an entire second style sheet where we :not() EVERY single css rule? For example what if you use a tree class in there or a dropdown or anything else it could still cause problems.

I think the other way to do this is just not put on the ids-enterprise rule on that top level element and that would serve as a not.

<div class='ids-enterprise'>
   <!-- Soho / IDS stuff -->
</div>
<div class='anything-else-but-ids-enterprise'>
   <!-- Not Soho / IDS stuff -->
</div>

@tfavorite
Copy link
Contributor Author

Thanks for looking into it @tmcconechy ! This and a couple other issues (which I will log) are preventing Infor Nexus from migrating to IDS (we're on a years-old forked version of Soho), so this would be a big help.

@tmcconechy
Copy link
Member

Cool yes enough use cases to add this. Will look into it soon. I think just requiring the top level element but may mean a 5.0 version just to follow semantic versioning with a small number of breaking changes.

@Fruko
Copy link

Fruko commented Oct 7, 2019

@tmcconechy but what about scenario like this

    <div class="accordion ids-enterprise" data-demo-set-links="true" data-options="{'allowOnePane': false}">
      <div class="accordion-header">
        <a href="#"><span>Warehouse Location</span></a>
      </div>
      <div class="accordion-pane">
        <div class="accordion-content">
          <some-component-with-own-styles></some-component-with-own-styles>
        </div>
      </div>

given code above, how can we make sure that ids-enteprise css won't mess with "some-component-with-own-styles?

From my point of view adding root element to cascade css from is a good idea for a start, but I believe in order to make enteprise css usable, there has to be a way of making it not fiddle with other parts of apps, if :not selector is not an option, what about prefixing all ids-enterprise classes with ids-enterprise?

@tmcconechy
Copy link
Member

tmcconechy commented Oct 7, 2019

yeah i thought about prefixing all options with ids- but its a much more breaking change. Also not sure that it would solve this exact case if we just made it called ids-accordion-content.

For this exact case i think we could come up with something that negates any styling in the accordion-content but i think that would be a separate issue. And would have to do this component by component.

Im just doubting there is any good way to make a not-ids class that works on every single entire component no matter where you put it. So it's going to be more of a case by case thing for that.

@tmcconechy
Copy link
Member

Wonder if a more simple reset would help tho? Fx... ids-reset

  • would set font, padding, margin and maybe a few more things back to document root?

Sort of a reverse reset? Might help in some cases but definitely not all?

@Fruko
Copy link

Fruko commented Oct 7, 2019

that might do the trick

@tmcconechy
Copy link
Member

Ok will take a look along with this. Thats not as bad as i was expecting initially.

@ahnpnl
Copy link

ahnpnl commented Oct 7, 2019

I like the idea of prefix with ids. Actually it's called "branding" concept. Definitely it's a breaking change but it should be the way to go for ids-enterprisd css structure in the future.

@tmcconechy
Copy link
Member

I agree have to weight the value over the breaking change.

@tmcconechy tmcconechy added this to To do in Enterprise 4.29.x (May 2020) Sprint via automation Apr 23, 2020
@tmcconechy tmcconechy added this to To do in Enterprise 4.30.x (June 2020) Sprint via automation Apr 30, 2020
@tmcconechy tmcconechy added the focus: refactor Label for tasks we should include for a major release refactor label May 22, 2020
@tmcconechy tmcconechy self-assigned this Jun 5, 2020
@tmcconechy tmcconechy added this to To do in Enterprise 4.31.x (July 2020) Sprint via automation Jun 15, 2020
@tmcconechy tmcconechy added this to To do in Enterprise 4.32.x (Aug 2020) Sprint via automation Jul 7, 2020
@tmcconechy tmcconechy added this to To do in Enterprise 4.33.x (Sep 2020) Sprint via automation Aug 3, 2020
@tmcconechy
Copy link
Member

tmcconechy commented Sep 22, 2020

Checked this further. It cant be done without a prefix added to the root. Was hoping i could find a way.
In the next gen project everything has prefixes and is self contained so this will not be an issue. Shelving this for now but not closing as we might need to do this on a major version

@tmcconechy
Copy link
Member

We have a solution in the 5.0 next version where each component is a web component with encapsulated css and all css is namespaced. This would satisfy this issue so closing

@tfavorite
Copy link
Contributor Author

@tmcconechy that's great news! Is there a good way to track when this is generally available?

@tmcconechy
Copy link
Member

You can keep an eye on this project https://github.com/infor-design/enterprise-wc -> it is a ways off but sometime next year should have something released

@tfavorite
Copy link
Contributor Author

Looks really promising!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus: refactor Label for tasks we should include for a major release refactor status: on hold ⏸️ type: breaking change 💥 type: enhancement ✨ [5] Velocity rating (Fibonacci)
Projects
None yet
Development

No branches or pull requests

4 participants