Skip to content
This repository has been archived by the owner on May 22, 2019. It is now read-only.

Adding withAITracking will break styling for components (e.g. full width/height) #73

Open
hanvyj opened this issue Mar 27, 2019 · 5 comments

Comments

@hanvyj
Copy link

hanvyj commented Mar 27, 2019

I just had an issue where I wrapped a component with withAITracking for a component that was scaled to the full width and height of it's parent (width: "100%"; height: "100%").

As the beta now wraps the componet in a div element for the tracking, and this div is unstyled, the resulting wrapped component no longer fills the screen.

@hanvyj hanvyj changed the title Adding withAITracking will break styling for components Adding withAITracking will break styling for components (e.g. full width/height) Mar 27, 2019
@hiraldesai
Copy link
Collaborator

Hi @hanvyj - AFAIK it is common for HOC to wrap the original component with a div. I have used it in other projects without it affecting the styles. Is it not something that you can work around by just changing your CSS rules?

I can add a custom class for the wrapper div but I'd rather consumers just worked around it by targeting elements accordingly.

@hanvyj
Copy link
Author

hanvyj commented Mar 28, 2019

You can work around it but it requires you to add some custom styling whever you use a component that's being tracked (with some components anyway).

Seems like it should be the responsibility of the component to style itself rather than rely on anything using it to remember and add some styling.

It just makes me uncomfortable having a situation where I have a reminder "Remember to add this style to the parent of this component or it'll break!" or something.

Ultimately, it's a simple workaround though. Up to you guys of you think it's not something to include? Happy for you to close the ticket if so.

Edit: Although I just had a bug where the 'tracked' component is sometimes in a child, sometimes not - not having the child control the div means the parent's styles .parent > div: { ... } then messes up other children - meaning I have to have either !important or wrap any child in an empty div, feels very hacky.

@hiraldesai
Copy link
Collaborator

Can you please advise what is your suggested workaround is? Thanks.

cc @pviotti - FYI.

@hanvyj
Copy link
Author

hanvyj commented Apr 2, 2019

I'm not sure if it would be the best option, but could you just pass through an optional class to set on the div?

withAITracking(component: Component, name?: string, class?: string)

I'll can try do a pull request if I've got some spare time and you think it would be a sensible change?

@florianchevallier
Copy link

Hi, I have the same problem here, did you find a way to solve it ?

florianchevallier added a commit to florianchevallier/react-appinsights that referenced this issue May 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants