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

Default classes for DIV elements #147

Closed
dsone opened this issue Mar 9, 2019 · 3 comments
Closed

Default classes for DIV elements #147

dsone opened this issue Mar 9, 2019 · 3 comments

Comments

@dsone
Copy link

dsone commented Mar 9, 2019

This issue is a: "Feature" request

Description

Currently, there are three DIV elements added to the body on zoom and you have no clue what DIV is the overlay/zoomContainer.
That leads to issues similar to #127.
I was using the zoomContainer for custom styling, thinking that was the right one. Apparently, it wasn't.

It would be nice if the DIV containers would contain default class names. This would make it so much easier to do basic styling outside of the provided options of the package itself and support better understanding of what is added into the DOM.

@rpearce
Copy link
Owner

rpearce commented Mar 10, 2019

Hi @dsone, thank you for voicing your concern! Let's see if I can address all the things.


Currently, there are three DIV elements added to the body on zoom and you have no clue what DIV is the overlay/zoomContainer.

image

At the moment, those first two are there for nothing more than React rendering purposes and hanging on to some events (probably could be done better). The one that is highlighted is the fixed container, and the div that is a sibling to img is the overlay.


It would be nice if the DIV containers would contain default class names. This would make it so much easier to do basic styling outside of the provided options of the package itself and support better understanding of what is added into the DOM.

While adding classnames specific to this library is not something I think is a good idea for a component, perhaps adjusting the API in the rewrite (which I've not gotten back to but is on my list of priorities) to encourage clear, fine-grained control is something I can think on as it gets rewritten.


As far as the image and zoomImage stand, at present you can pass them your own className props, and for control over the overlay and even more, you can pass defaultStyles to override these styles


I'll keep this issue open until the rewrite occurs.

@rpearce
Copy link
Owner

rpearce commented Oct 6, 2019

Hi, we're not done with the rewrite yet, but we are keeping this in mind for the following rewrite issue: #165.

@rpearce
Copy link
Owner

rpearce commented Dec 18, 2019

As you can see when inspecting a zoomed image here, https://rpearce.github.io/react-medium-image-zoom/?path=/story/react-medium-image-zoom--with-img, we are using some class names for static styling (see image):

image

The API for v4 allows for:

  • overlay background starting color (before transition)
  • overlay background ending color (after transition)
  • animation transition duration
  • ability to set the zoomMargin
  • ability to pass a wrapStyle object to add some last-minute styles to the top-level div that we use to wrap the other components

While v4 is still in alpha, I'm going to close this PR, for I believe it has been addressed there and will help to clean up the board. If you feel differently, please let me know.

@rpearce rpearce closed this as completed Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants