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.

Sign up for GitHub

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

Jump to bottom

[Image] "link" images are not centered #1608

Closed
layershifter opened this issue Jul 27, 2020 · 8 comments
Closed

[Image] "link" images are not centered #1608

layershifter opened this issue Jul 27, 2020 · 8 comments
Assignees
@ko2in
Labels
lang/css Anything involving CSS type/bug Any issue which is a bug or PR which fixes a bug
Milestone
2.8.7

Comments

@layershifter
Copy link
Contributor

Bug Report

The issue was originally reported in Semantic-Org/Semantic-UI#4708 and then in Semantic-Org/Semantic-UI-React#3953.

Steps to reproduce

  1. Open a Fiddle
  2. Mention that an image on bottom is not centered.

Expected result

All images are centered.

Actual result

Link images are not centered.

Testcase

https://jsfiddle.net/epc4nL2a/

Screenshot (if possible)

image

Version

2.8.6
@layershifter layershifter added state/awaiting-investigation Anything which needs more investigation state/awaiting-triage Any issues or pull requests which haven't yet been triaged type/bug Any issue which is a bug or PR which fixes a bug labels Jul 27, 2020
@layershifter layershifter mentioned this issue Jul 27, 2020
Closed
@ko2in ko2in removed state/awaiting-investigation Anything which needs more investigation state/awaiting-triage Any issues or pull requests which haven't yet been triaged type/bug Any issue which is a bug or PR which fixes a bug labels Jul 27, 2020
@ko2in
Copy link
Member

ko2in commented Jul 27, 2020

@layershifter , You don't need to specify centered image to your link. Here's fiddle: https://jsfiddle.net/sqr50cjh/

@layershifter
Copy link
Contributor Author

@ko2in As I see you moved all CSS classes to img instead of a, however an example in SUI/FUI docs does the opposite: https://fomantic-ui.com/elements/image.html#image-link.

It feels confusing...

@ko2in
Copy link
Member

ko2in commented Jul 27, 2020 •
edited
Loading

@layershifter You're right that the doc looks confusing. I guess you should provide specific rule to your link to display as block if you're willing to present the image as a link and align center.

Currently, the center alignment only applies to img because it's displaying as block element as standalone. Maybe, we should explicitly set as block element for all .ui.centered.image.

I'll check later if I should submit PR for that.

@lubber-de
Copy link
Member

@ko2in here is a quick jsfiddle. Providing display:block for any centered image seems to fix the issue.
However, for image groups it still does not work
https://jsfiddle.net/lubber/vu4bh3g1/5/

@ko2in
Copy link
Member

ko2in commented Jul 27, 2020

@lubber-de Maybe, this should work: https://jsfiddle.net/spq9kLd3/

@lubber-de
Copy link
Member

@ko2in Looks good! Will you prepare a PR?

@lubber-de lubber-de added lang/css Anything involving CSS type/bug Any issue which is a bug or PR which fixes a bug labels Jul 27, 2020
@lubber-de lubber-de added this to the 2.8.7 milestone Jul 27, 2020
@ko2in
Copy link
Member

ko2in commented Jul 27, 2020

Yes. I'll do more investigations and prepare PR.

ko2in added a commit to ko2in/Fomantic-UI that referenced this issue Jul 29, 2020
@ko2in
fix(image): Support the image to be centered when using a different H…
bd2c704 
…TML element

The current image component can only be centered with img element. It is not able
to center when using a different HTML element, for example, an image link with
anchor tag or a DIV container.

This PR would support to center the image not only for img tag, but also for other
HTML element by using `centered` flag.

This PR would also support to display the group images horizontally center. Additionally
the group images can now share the same vertical alignment as `top`, `middle` or `bottom`
together or can have it's own vertical alignment.

Closes: fomantic#1608
@ko2in ko2in mentioned this issue Jul 29, 2020
Merged
@ko2in ko2in added the state/has-pr An issue which has a related PR open label Jul 29, 2020
@lubber-de
Copy link
Member

Fixed by #1618

@lubber-de lubber-de added tag/next-release/nightly Any issue which has a corresponding PR which has been merged and is available in the nightly build and removed state/has-pr An issue which has a related PR open labels Jul 30, 2020
@lubber-de lubber-de removed the tag/next-release/nightly Any issue which has a corresponding PR which has been merged and is available in the nightly build label Sep 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ko2in ko2in

Labels
lang/css Anything involving CSS type/bug Any issue which is a bug or PR which fixes a bug
Projects
None yet
Milestone
2.8.7
Development

No branches or pull requests
3 participants
@ko2in @layershifter @lubber-de