stephane-monnot / react-vertical-timeline

Vertical timeline for React.js
https://stephane-monnot.github.io/react-vertical-timeline/
MIT License
1.09k stars 161 forks source link

VerticalTimelineElement doesn't properly center all icon elements #173

Open 25zeeshan opened 10 months ago

25zeeshan commented 10 months ago

Problem Description: The VerticalTimelineElement currently centers only SVG icon elements while causing overflow issues for other types of elements, like img. The current CSS specifically targets SVGs, resulting in sizing and overflow discrepancies for other icon types.

Proposed Solution: Requesting a modification in the CSS selector from **.vertical-timeline-element-icon svg** to *`.vertical-timeline-element-icon `**, which will apply the centering styles universally to all elements inside the .vertical-timeline-element-icon class.

Reproduction: For a detailed reproduction run this project in local, github repository - https://github.com/25zeeshan/react-timeline-bug

Visuals:

The icon props here is - <img src="https://fastly.picsum.photos/id/5/200/200.jpg?hmac=oN9VtXdJYLSFssji8vCr48JaI-e5Zi4eH9GAiYBB_Ig" alt="test Imae" />

With current css properties - scrnli_03_01_2024_02-41-54

After changing svg to * - scrnli_03_01_2024_02-44-27

25zeeshan commented 10 months ago

Hi @stephane-monnot ,

Fixed the issue and requested a PR #174. Please review for merge.

Thanks

timtucker-dte commented 7 months ago

Even SVG centering doesn't work 100% right.

Far simpler to just put the following -- then the child element doesn't even need any extra positioning rules:

.vertical-timeline-element-icon {
    display: grid;
    place-items: center;
}
25zeeshan commented 7 months ago

Thanks for your suggestion @timtucker-dte. But another problem is sizing of the svg or img element (whatever we want to place as icon)

if we just use grid with place-items: center , the image completely covers the .vertical-timeline-element-icon if image is not sized.

React Vertical Timeline – Demo

This is the expected results: React Vertical Timeline – Demo (1)

Your suggested code works if we specify the sizing for children of .vertical-timeline-element-icon

.vertical-timeline-element-icon *{
  width: 24px;
  height: 24px;
}

Also, you mentioned it doesnt work 100% for svg, can you share the scenario where my change does not work??

timtucker-dte commented 7 months ago

@25zeeshan Missed that because I'm also overriding the sizing. In my case, I'm specifying a width and then auto for height since not all of my icons are square.

What I ran into was that icons were being centered horizontally but not vertically until I switched to using display: grid.

Rather than having a fixed height and width, it might be better to specify a max-width that's less than the size of the container and then set height to auto to preserve aspect ratio.

Even in your own screenshot, you can see that the bag icon is shifted downwards in the second image.