mgsisk / inkblot

Inkblot is an elegant, fully responsive, highly customizable Webcomic-ready WordPress theme.
http://wordpress.org/themes/inkblot
45 stars 11 forks source link

Two small bugs: Header de-centers on full-screen, and mobile menu can display a nonexistent page. #43

Closed Victorre closed 9 years ago

Victorre commented 9 years ago

Hello, me again. While I've been working with Inkblot to prepare my site I've noticed two things that seemed like bugs that I thought you might want brought to your attention, if someone hasn't already. Best regards.

HEADER When I resize my browser window to nearly fullscreen or more the Header Image aligns slightly, but not fully, right. Putting this code into the child theme fixed the issue, but also necessarily shrunk the banner (which wasn't a big deal for me personally, as I just resized mine bigger in Photoshop):

.banner { margin-left: auto; margin-right: auto; width: 85%; float: none; }

MENU When I resize the site is to mobile-size the menu shrinks, and is reduced to a single item for all pages (i.e. “Home”). When I then click on a new page in my menu the single item shown also changes to reflect the new open page. So far so good.

However, I also have items in my menu that aren’t linked to another page (they have a URL of “ # ”) but are simply the headers of drop-down submenus. When I click on one of them that item becomes the header shown in the menu, even though the page hasn't changed. So I wind up at my "Home" page, with a heading like "About".

Victorre commented 9 years ago

(Oh, it bears mentioning that to check these I temporarily deleted all of my extra CSS, to make sure it wasn't just my alterations causing the problem.)

mgsisk commented 9 years ago

Thanks @Victorre. Can you share a screenshot of what you're seeing with the header image? I can't seem to replicate this. You should be able to remove float: none; on .banner, though, as (by default, at least) .banner isn't floated.

For the menu, it's technically working as it should. When a responsive width is set the site menu converts from a ul to a select at the the responsive width to save space on smaller screens. This menu is "dumb," however, in the sense that it just lists out all of the menu items, regardless of their intent on the full site display. I'm not sure there's a great way to get around this one, really; I could alter the walker to check for # URL's and either not render them or forcibly ignore it when they're selected, but I can imagine instances where such URL's would exist in the site nav and still need to be selectable.

Victorre commented 9 years ago

Sure, I've emailed two screenshots to you at your listed address (I couldn't get them to attach to the forum post). One with the CSS fix, one without.

While I was taking the screenshots I discovered that even with the fix it still de-centered when the window was made even more large than I must have tested. I was able to solve that by reducing the size to 80%, so there seems to be a direct connection between the percentage by which the image is reduced and how big my browser window can get before it de-centers.

The float, as you pointed out, actually has no effect, so I removed it without consequence.

Victorre commented 9 years ago

I forgot to mention, in case it's relevant, that the error occurs in both Firefox and Safari. I just checked and my versions are up to date. My display dimensions (according to an online checker) are 20 x 11.3, with a resolution of 1920 x 1080.

mgsisk commented 9 years ago

Have you tried adding text-align: center; to .banner, like:

.banner {
    text-align: center;
}

You should be able to remove the other styles and just use that to keep the banner image centered at any resolution.

Victorre commented 9 years ago

Cool, that worked. Thanks.