-
-
Notifications
You must be signed in to change notification settings - Fork 94
Add the possibility to provide a header image and add Dresden.rb logos #1136
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
app/helpers/home_helper.rb
Outdated
| File.join('labels', Whitelabel[:label_id], 'header.png') | ||
| end | ||
|
|
||
| def header? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO this method name is to generic and doesn't communicate the intend. How about header_image_available?, uses_header_image?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree those names are much better!
app/helpers/home_helper.rb
Outdated
| end | ||
| end | ||
|
|
||
| def header_path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| def header_path | |
| def header_image_path |
|
In general I like the idea. Result on Mobil looks "strange", but if that is fine for you. |
593ac44 to
da24ac7
Compare
Mh π€ To me it looks exactly the same as the other logo + heading variant? Both are just hidden on mobile or not? Maybe there is something different on my device? |
was thinking about the small logo in the header, in the top left corner. For me, it felt like a little bit hard to recognise. |
Oh I see! Well I guess this is related to the the logo itself, which does not set itself apart from the background to well. I'll see if I can make it a bit bigger so that it's more prominent. |
da24ac7 to
eb34f4a
Compare
I increased the logo size a bit which should be the best we can do for now. |
|
@salzig Sorry for pinging, anything we can do to get this merged? π |


Hey! We now have logos π§βπ³
Because I really like the typography that comes with our logo (see the little ruby as the dot!) I thought it might be a cool idea to provide the possibility to add a header image instead of the logo + h1 combo that remains the default of course. Let me know what you think about this! If you don't like that it's fine and I'm gonna close this PR and create a new one to just change our logo.
This PR also fixes the favicon of our meetup page.