Issue 2617 - Remove GmbH from logo and make it compatible with small screens [eyeo.com]

Related issue:
https://issues.adblockplus.org/ticket/2617

[saroyanm, 2015-06-02 10:56:03]

Thomas can you please have a look, previously the menu items were breaking to
new line when I was resizing the screen to 431px width, with current layout
adjustment from 397px width. 
Anyway would be nice to have different layout for header so it will have no
adjustment problem dependent from header menu items quantity, but that I would
consider as separate fix for future, I'll create a bug ticket for that.

[Thomas Greiner, 2015-06-03 14:51:05]

On 2015/06/02 10:56:03, saroyanm wrote:
> Thomas can you please have a look, previously the menu items were breaking to
> new line when I was resizing the screen to 431px width, with current layout
> adjustment from 397px width. 
> Anyway would be nice to have different layout for header so it will have no
> adjustment problem dependent from header menu items quantity, but that I would
> consider as separate fix for future, I'll create a bug ticket for that.

#2617 mixes two issues which have nothing to do with each other. Therefore I
would split-off the style change into a separate ticket and only consider the
image change here.

Furthermore I don't see much benefit in this temporary style change because it
neither eliminates the horizontal scrolling nor the text being cut off.
Therefore let's work on a more long-term fix right away instead of introducing
this workaround.

[saroyanm, 2015-06-03 15:00:29]

On 2015/06/03 14:51:05, Thomas Greiner wrote:
> On 2015/06/02 10:56:03, saroyanm wrote:
> > Thomas can you please have a look, previously the menu items were breaking
to
> > new line when I was resizing the screen to 431px width, with current layout
> > adjustment from 397px width. 
> > Anyway would be nice to have different layout for header so it will have no
> > adjustment problem dependent from header menu items quantity, but that I
would
> > consider as separate fix for future, I'll create a bug ticket for that.
> 
> #2617 mixes two issues which have nothing to do with each other. Therefore I
> would split-off the style change into a separate ticket and only consider the
> image change here.
I think this both changes somehow related in the context of current header
implementation,
while the new logo has different size I think we need to also adjust the style
for that.
> Furthermore I don't see much benefit in this temporary style change because it
> neither eliminates the horizontal scrolling nor the text being cut off.
> Therefore let's work on a more long-term fix right away instead of introducing
> this workaround.
This solution at least doesn't let the header menu items to break to another
row. I would consider this as quick fix to current issue we have and
re-implement header layout with separate issue, because this will need more time
to implement because as far as I can see long term solution needs just new
layout, also make sense to align with designers for new layout before
implementing them. 
Please let me know if you still think that this quick solution is useless.

[Thomas Greiner, 2015-06-03 16:11:16]

On 2015/06/03 15:00:29, saroyanm wrote:
> I think this both changes somehow related in the context of current header
> implementation,
> while the new logo has different size I think we need to also adjust the style
> for that.

The one is about removing the "GmbH" from the logo and the other is about
optimizing the header for smaller screens but the second one is not accomplished
with this code.

> This solution at least doesn't let the header menu items to break to another
> row. I would consider this as quick fix to current issue we have and
> re-implement header layout with separate issue, because this will need more
time
> to implement because as far as I can see long term solution needs just new
> layout, also make sense to align with designers for new layout before
> implementing them. 
> Please let me know if you still think that this quick solution is useless.

I really don't think we need to involve a designer for fixing something. From
what I see the fix will basically involve getting rid of `display: table-cell`
and making the menu items flow vertically.

If, however, you want a temporary solution in this very review then let's at
least limit it to the `margin-left` value change because the others are purely
cosmetical.

[saroyanm, 2015-06-03 16:34:31]

On 2015/06/03 16:11:16, Thomas Greiner wrote:
> On 2015/06/03 15:00:29, saroyanm wrote:
> > I think this both changes somehow related in the context of current header
> > implementation,
> > while the new logo has different size I think we need to also adjust the
style
> > for that.
> 
> The one is about removing the "GmbH" from the logo and the other is about
> optimizing the header for smaller screens but the second one is not
accomplished
> with this code.
Well what I meant by title is making the logo compatible with smaller screens,
anyway I can think of better title for the issue.
> > This solution at least doesn't let the header menu items to break to another
> > row. I would consider this as quick fix to current issue we have and
> > re-implement header layout with separate issue, because this will need more
> time
> > to implement because as far as I can see long term solution needs just new
> > layout, also make sense to align with designers for new layout before
> > implementing them. 
> > Please let me know if you still think that this quick solution is useless.
> 
> I really don't think we need to involve a designer for fixing something. From
> what I see the fix will basically involve getting rid of `display: table-cell`
> and making the menu items flow vertically.
Yes I also was thinking about that solution, but please note that solution will
decrease the view port of Eyeo image. Also I was thinking how it would be better
to present the green button, should it stay in it's place or again it should be
under the eyeo logo (this is something make sense to decide under separate issue
of layout fix). 
> If, however, you want a temporary solution in this very review then let's at
> least limit it to the `margin-left` value change because the others are purely
> cosmetical.
Well cosmetic changes will make at least the website look Okeyish under the
mobile, as I mentioned I can add the small cosmetic changes information in the
ticket and also mentioning that this is temporary solution.

[Thomas Greiner, 2015-06-03 16:57:21]

On 2015/06/03 16:34:31, saroyanm wrote:
> Well what I meant by title is making the logo compatible with smaller screens,
> anyway I can think of better title for the issue.

There's no need to overcomplicate things so please leave the styling part out of
the ticket. First we need to get the logo fixed and then work on the layout
issue.

> Well cosmetic changes will make at least the website look Okeyish under the
> mobile, as I mentioned I can add the small cosmetic changes information in the
> ticket and also mentioning that this is temporary solution.

There's no need to make cosmetic changes if it will be covered in a follow-up
optimization anyway. The reason for having a temporary solution in the first
place is to make it usable, not to make it pretty.

[saroyanm, 2015-06-04 18:23:16]

Updated

[Thomas Greiner, 2015-06-04 22:12:26]

LGTM