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

Secondary Tag CSS alignment off in discussion list #3245

Closed
zerosonesfun opened this issue Jan 1, 2022 · 7 comments · Fixed by flarum/tags#160
Closed

Secondary Tag CSS alignment off in discussion list #3245

zerosonesfun opened this issue Jan 1, 2022 · 7 comments · Fixed by flarum/tags#160

Comments

@zerosonesfun
Copy link
Contributor

zerosonesfun commented Jan 1, 2022

Bug Report

Current Behavior
The secondary tags are slightly misaligned on small screens.

Steps to Reproduce

  1. With a mobile device, go to discuss.flarum.org and look for posts with multiple tags.

Expected Behavior
Tags should be perfectly aligned in a row.

Screenshots
screenshot

Environment

  • Flarum version: 1.1.1

Possible Solution
I tried a couple basic tweaks such as adding margin-top to child tags, but even with !important I'm unable to correct the misalignment. Admittedly though, I did not spend a ton of time on this. Maybe later I can, unless this is already fixed for the next release.

@davwheat
Copy link
Member

davwheat commented Jan 1, 2022

Thanks for the bug report!

Discuss is currently on a preview build of the next version, so if it's happening on there, it's not been fixed yet.

I can't reproduce this personally. What device and browser are you using?

Screenshot_2022-01-01-14-44-13-012_com android chrome

@zerosonesfun
Copy link
Contributor Author

zerosonesfun commented Jan 1, 2022

This happens on Apple devices (of course). Mac and iPhone. Only small screens/the mobile layout. And, even stranger, if the secondary tag is long, it doesn’t happen. The tag name has to be fairly short. For example, I have a secondary tag called Podcast Episodes. Because that’s fairly long it’s aligned. But other tags like Meta, are not aligned.

Side note, I feel like since Apple released their most recent MacOS and iOS, I’ve seen more browser issues than I have in a long time.

Maybe if no one else reports this, or no one else cares that much, we just wait and see if maybe Apple’s next releases fix things. Because I’m sure they’ve received all kinds of bug reports.

@zerosonesfun
Copy link
Contributor Author

zerosonesfun commented Jan 20, 2022

Someone confirmed this is a thing:

https://discuss.flarum.org/d/29882-tags-appear-broken-in-safari/8

Fix is:

.TagsLabel > * {
vertical-align: bottom;
}

I will attempt a pull request.

@zerosonesfun
Copy link
Contributor Author

zerosonesfun commented Jan 23, 2022

New idea. Forget the > * part, it seems like this works without affecting other browsers:

.TagLabel {
vertical-align: bottom;
}

I'm not going to do a pull request after-all. This is just one CSS rule that needs to be added. Any core dev can do this at any moment in a flash.

Someone just needs to make sure adding vertical-align: bottom won't hurt anything in other browsers. All looks fine to me.

@askvortsov1
Copy link
Sponsor Member

New idea. Forget the > * part, it seems like this works without affecting other browsers:

.TagLabel {
vertical-align: bottom;
}

I'm not going to do a pull request after-all. This is just one CSS rule that needs to be added. Any core dev can do this at any moment in a flash.

Someone just needs to make sure adding vertical-align: bottom won't hurt anything in other browsers. All looks fine to me.

It would still be preferable to do this via a PR to better facilitate discussion (we usually try to avoid pushing directly to main, even as core devs). From a quick local test, it seems to work though!

@zerosonesfun
Copy link
Contributor Author

New idea. Forget the > * part, it seems like this works without affecting other browsers:

.TagLabel {

vertical-align: bottom;

}

I'm not going to do a pull request after-all. This is just one CSS rule that needs to be added. Any core dev can do this at any moment in a flash.

Someone just needs to make sure adding vertical-align: bottom won't hurt anything in other browsers. All looks fine to me.

It would still be preferable to do this via a PR to better facilitate discussion (we usually try to avoid pushing directly to main, even as core devs). From a quick local test, it seems to work though!

Ok, pull request done. Thanks for taking a look!

@zerosonesfun
Copy link
Contributor Author

Sorry @davwheat and @askvortsov1 but I noticed today on my iPhone that the CSS I ended up using in this pull request doesn't work. I don't know what's going on. Could be extension conflicts. Could be caching issues. The only way I can fix my tag alignment issue in mobile Safari is by using:

.TagsLabel > * {
vertical-align: bottom;
}

The >* part seems to be needed after all. For me at least. And I did not include this part in the pull request.

But, at this point. I just don't know. You might want to undo this pull request/merge even though it probably won’t harm anything. Because, it probably won’t help either.

You might need someone more experienced with an iPhone to fully debug and figure out this one. It's tough because I obviously do not have a clean local dev environment on my iPhone. So this can only be tested on live sites with a bunch of extensions.

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

Successfully merging a pull request may close this issue.

3 participants