-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Minor tweaks to the welcome page #33510
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
Conversation
It's better, thank you. Some more comments:
|
55f66e7
to
eacb97a
Compare
Status: Needs Work |
f5ec6fc
to
ddffc97
Compare
@javiereguiluz @yceruto First, thanks for this new project page design, it's awesome! Concerning the alignment issues, I could solve them for all browsers quite easily using Flex (not talking about Symfony's one 😄). Is there any reason you didn't use these CSS properties? It also makes CSS code cleaner in my opinion. I would be happy to share it if you wish. Please note that I didn't test it on IE11 (if we really need this...), but that should be doable to make it work anyway if it doesn't already. |
I don't think supporting IE 11 for this page is needed. Hopefully web developers don't use it as their main browser. |
Yeah I totally agree as we are targeting developers. I will have a quick look just by curiosity. UPDATE: removed my diff post as I was comparing to the current 4.4 branch, and not this PR branch. |
@michaelperrin I can't see it correctly with Firefox. Look at how different is the vertical align of the monospaced text. The problem is that both browsers are using different fonts ... because we don't define the font family explicitly, so I could solve the problem by setting the font family explicitly. I used the same font stack as Bootstrap's monospace:
|
You're right. My CSS flex solution wouldn't help for this. I checked with the Bootstrap's monospace font family and it makes the correctly aligned indeed. |
@javiereguiluz Let's use the same font everywhere as done with Bootstrap. I don't see any drawback. |
Thank you @yceruto. |
This PR was merged into the 4.4 branch. Discussion ---------- Minor tweaks to the welcome page | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #33189 (comment) | License | MIT | Doc PR | - /cc @fabpot Commits ------- acd5061 Some styling tweaks ddffc97 minor tweaks to the welcome page
Thank you @javiereguiluz ❤️ |
This PR was merged into the 4.4 branch. Discussion ---------- Minor updates in the new Welcome page | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - After #33510 was merged, there were some concerns about the warning message in the "Welcome Page". It's too low on the page, so it's easy to oversee it. So, in this PR we propose to make the warning message way more visible and other minor tweaks. This is how it'd look on a smartphone and on a desktop:  Commits ------- 4517319 Minor updates in the new Welcome page
/cc @fabpot