Skip to content

Conversation

@IceSentry
Copy link
Contributor

@IceSentry IceSentry commented Mar 30, 2022

So that was pretty easy. Everything was already setup correctly except that the max-width was slightly too wide to allow flex to do it's thing and show the features side by side.

I also moved the image for the cross platform section inside the container.

image

Closes #308

@IceSentry IceSentry added C-Feature A new feature, making something new possible A-Home C-Webdev labels Mar 30, 2022
@alice-i-cecile
Copy link
Member

Pretty! Can you get a new screenshot with "Features" removed?

@IceSentry IceSentry changed the title update feature width Show feature image next to text Mar 30, 2022
@IceSentry
Copy link
Contributor Author

I updated the screenshot

@IceSentry
Copy link
Contributor Author

IceSentry commented Mar 30, 2022

I made a small change to how we do this. Instead of relying on adding a container-reverse class manually I used the css selector ::nth-child(odd) which will apply the wrap-reverse property automatically on each odd children. This makes it so we don't need to remember to add the class.

It has 95.28% browser support on caniuse so I think it's not an issue https://caniuse.com/mdn-css_selectors_nth-child

@rparrett
Copy link
Contributor

What sort of device are you testing with?

Both 28rem and 30rem seem pretty arbitrary to me, and I have always experienced the features page as "alternating side by side."

But I don't think that this makes the images too small, and if there's some common screen size that now displays the page side-by-side then that's good.

@IceSentry
Copy link
Contributor Author

IceSentry commented Mar 30, 2022

Interesting, I'm just using an old 1080p desktop monitor and I also tried it on a 1440p ultrawide monitor and they both show everything lineraly like this:

image

image

That's using vivaldi (so chromium) with windows 10.

I haven't tried it in other browsers though.

I agree that the numbers seem arbitrary. I tried a few combinations and 28 was the one that worked.

There's a different approach that uses float left/right instead of using flex and hoping it wraps correctly.

@IceSentry
Copy link
Contributor Author

Oooh, interesting, so in edge and firefox it does work correctly without any changes.

@IceSentry
Copy link
Contributor Author

Chrome seems fine too...

@IceSentry
Copy link
Contributor Author

IceSentry commented Mar 30, 2022

Alright, found the issue. Apparently my default font size was set to 18 instead of 16 and that just broke everything.

@alice-i-cecile
Copy link
Member

@IceSentry what do you think we should do with this pr now that you've tracked down the root cause?

@IceSentry
Copy link
Contributor Author

Well, I'm trying to find ways that don't rely on a width proportional to the font size. It makes sense sometimes, but I'm not sure it's the best approach here.

There's an option to use float left and float right, but it doesn't adjust correctly on mobile.

Technically, this PR also fixed an issue where one of the image is not in a container which seems like an oversight to me.

So, I'll keep it open a bit, but if I don't find anything better I'll just close it.

@doup
Copy link
Contributor

doup commented Apr 2, 2022

After the changes to improve the header (#315) I've slightly broken the typography on my branch, which at the same time has broken the features page. Since I need to fix it, I'll improve the code and fix this issue also.

(👇 broken layout on my branch)
image

@doup
Copy link
Contributor

doup commented Apr 2, 2022

I've fixed this on #315, please take a look at the video.

@IceSentry
Copy link
Contributor Author

This will be fixed by the multiple layout improvements of @doup so I'll close this.

@IceSentry IceSentry closed this Apr 3, 2022
@IceSentry IceSentry deleted the compress-features branch April 7, 2022 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Home C-Feature A new feature, making something new possible C-Webdev

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compress Feature page layout

4 participants