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

VerticalTimelineElement doesn't properly center all icon elements #173

Open
25zeeshan opened this issue Jan 2, 2024 · 4 comments
Open

Comments

@25zeeshan
Copy link

25zeeshan commented Jan 2, 2024

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
Copy link
Author

Hi @stephane-monnot ,

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

Thanks

@timtucker-dte
Copy link

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
Copy link
Author

25zeeshan commented Apr 12, 2024

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
Copy link

@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.

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