The default search block could use some help. There is a lot of unused space on the right side of the block. I would recommend either making the textfield fill the width of the block, or make the search button and the text field inline with each other.

Attached screenshot shows that it currently looks like.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

watsonerror’s picture

Assigned: Unassigned » watsonerror

making form buttons in the sidebars a little smaller

watsonerror’s picture

smaller buttons and text fields in sidebars

watsonerror’s picture

Status: Active » Needs review
Jeff Burnz’s picture

Status: Needs review » Needs work

The changes are good, can you please make it work for the footer columns also. The search button looks a little scrunched but imo its better than before with it wrapping under.

FYI your patch index is wrong, it needs to be Index: css/style.css, I changed it to get the patch to apply.

watsonerror’s picture

thanks,
Jenn suggested using a magnifying glass icon like the one in the Firefox search bar.
I'm going to try that and post a screen-shot. That way the other buttons in the sidebar aren't affect by this change.

watsonerror’s picture

Status: Needs work » Needs review
FileSize
955 bytes

@Jeff Burnz
Button change applied to footer too. I also set a min width of the button for shorter words so the button doesn't begin to appear round.

Jeff Burnz’s picture

Are you gonna go for the magnifying glass icon idea - that sounds like it would work out really nice with this theme.

eojthebrave’s picture

We'll either need to find a GPL icon, or someone will have to make one. Which, I suppose wouldn't be to hard. Just a circle with line coming out of it.

watsonerror’s picture

yes I'll make the icon myself.

Jacine’s picture

Status: Needs review » Needs work

changing this to needs work, while waiting for the icon :)

watsonerror’s picture

FileSize
1.06 KB

should I add the button background to it? I would have to make it darker.

watsonerror’s picture

I'm going to add the button background to it. It will look a lot better. attachment to come later tonight.

eojthebrave’s picture

Nice! I like the icon. Good work.

I think we should add some javascript that ads the word 'Search' to the field, then automatically clears it when the field gets focus. Like the search field here for example: http://digg.com/

Something like the following should do the trick (works in D6, haven't tested in D7).

Drupal.behaviors.bartik = function(context) {
  $('#edit-search-block-form-1').val('Search');

  $('#edit-search-block-form-1').focus(function(){
    $(this).addClass('has-focus');
    if ($(this).val() == 'Search') {
      $(this).val('');
    }
  });

  $('#edit-search-block-form-1').blur(function(){
    $(this).removeClass('has-focus');
    if ($(this).val() == '') {
      $(this).val('Search');
    }
  });
}
bleen’s picture

FileSize
1.92 KB

This patch adds some javascript to implement @eojthebrave's suggestion ... its a bit different then the code in #13

bleen’s picture

@drhino ... is the patch in #6 supposed to be combined with the patch in #2, or is the patch in #6 supposed to be tested instead of #2?

In general if you submit a patch (like #2) and someone suggests you add to it, then next patch (like the one in #6) should include everything you did already plus the code you're adding.

Once I know what you are intending in #2 & #6 we can combine everything from all the patches.

Also, watch out for white space when you're writing code:

+++ css/style.css	25 Apr 2010 14:03:25 -0000
@@ -845,6 +845,21 @@ textarea.form-textarea,
+.region-sidebar-second input.form-text { ¶
+	width:58%;

no whitespace at the end of lines ... and use two-spaces instead of tabs (http://drupal.org/coding-standards)

Powered by Dreditor.

eojthebrave’s picture

@bleen18, awesome thanks!

Only comment on the javascript, we should probably use the javascript translation function. Drupal.t() for the word 'Search'. http://drupal.org/node/323109

Otherwise looks good to me.

bleen’s picture

FileSize
1.95 KB

good point ... here we go

watsonerror’s picture

FileSize
1017 bytes

check it out, this makes me happy.

watsonerror’s picture

Does anyone know what the blue area of the buttons sprite is for?

tlattimore’s picture

FileSize
4.92 KB

Looking really good in the screenshots drhino. It's not quite looking the same on my end, I attached a screenshot of what it looks like for me. Could you roll a single patch against HEAD that includes all your changes to this. Also, could you attach a copy of the image you used for the magnifying glass?

watsonerror’s picture

I was trying to hack the IE bug fix in but I figured there's a higher probability to fix this if I send out the patch.

watsonerror’s picture

oh I didn't clean up the css as requested earlier, sorry about that

watsonerror’s picture

I fixed the IE bugs new patch to come soon

watsonerror’s picture

Status: Needs work » Needs review
FileSize
1.95 KB

IE bug fixed

tlattimore’s picture

Hey drhino, would really like to review the patch from #21, but I cant seem to get the patch to apply? I don't see any meta information pointing the bartik CVS repo? It looks like it is point to a git repository to check diff, odd.

I am still pretty new to creating and applying patches, am I doing something wrong?

bleen’s picture

Status: Needs review » Needs work

The patch needs to be created from the root of the bartik folder ...

Also, once i manually edited the patch to try and review, it wouldn't patch properly... I think you need to update your version of Bartik from CVS Head and then reroll the patch.

Just looking at the code though .. this looks real close :)

watsonerror’s picture

Title: Improve formatting of default search block » CVS error

Cool I'll get this out today, but I kept getting an error when checking out bartik from CVS.

bleen’s picture

Title: CVS error » Improve formatting of default search block

Let keep the issue title on task ... Ill try and find you in IRC (or you can always ask jen or jacine or johnalbin or lots of others about the error if Im not around

JohnAlbin’s picture

Status: Needs work » Needs review
FileSize
2.65 KB

Here's a re-rolled patch.

JohnAlbin’s picture

Hmm… this is very close to being perfect, IMO. But I think the text box needs to expand depending on the size of the region its in.

Here's a patch that accomplishes that.

JohnAlbin’s picture

Status: Needs review » Fixed
watsonerror’s picture

thanks JohnAlbin!

bleen’s picture

Status: Fixed » Needs work

The search button is missing in iE6 and its half missing in IE6 RTL ... iE6 makes me sad

jensimmons’s picture

Assigned: watsonerror » Unassigned

The search block looks awesome on some pages (like the home page), but on others there's too much space on the bottom.

And then, the search *page* is kind of a mess.

Jeff Burnz’s picture

IE7 also, somethings really wierd here, try editing a node and the button will show partly, but in other views it will not show, the block is changing height also (I did say weird...)

Jeff Burnz’s picture

Component: User interface » Browser Compatibility
Category: task » bug
Status: Needs work » Needs review
FileSize
19.69 KB
2.06 KB

Heres a different approach that uses position absolute instead of floating the div, which is the main problem here.

Needs RTL love...

Screenshot is IE7.

bartik_search_button_ie7.png

tlattimore’s picture

Looks good in IE6-7. But as stated, RTL needs work. It's doing some really quirky things there.

Jeff Burnz’s picture

At this eleventh hour I am not sure we need to nail all RTL issues, they are mostly minor, even 7 has major RTL issues as I recall. Its just a matter of a left/right here and there.

JohnAlbin’s picture

Status: Needs review » Needs work

Oh, wow. I don't like position: absolute for this at all. Why do you think float doesn't work here?

Jeff Burnz’s picture

Absolute is cool for positioning input buttons because you can lock it to the end of the text field, if the field gets wider it simply moves over (auto left/right values), but it will never wrap, which is bad and might break out/bleed over the edge of the block border etc, so yes, its bad-ass in that regard.

Frankly other positioning methods are more flexible and more well suited for this, I just lacked time to work it up properly and threw this out there for testing and feedback - just brain storming for the IE fix really.

Jeff Burnz’s picture

I'm looking in IE7 and not seeing an search button on the search block.

Jeff Burnz’s picture

Priority: Normal » Critical
FileSize
1.01 KB

Heres a screenshot, this is critical.
ie7-search-block.gif

jensimmons’s picture

Priority: Critical » Normal

This needs to get cleaned up, but it's not a critical bug. It's more of a must-get-to-this than a this-makes-things-explode.

jensimmons’s picture

Project: Bartik » Drupal core
Version: 7.x-1.x-dev » 7.x-dev
Component: Browser Compatibility » Bartik theme
jensimmons’s picture

Here's a weird problem, the search block has more padding on the bottom on some pages on the site than on others.

Home page, page page:

Article node page, Contact Form page:

What's up with that? Let's fix it.

bleen’s picture

...and if you look really close at the screenshots in #45, there is more space between the input and the button in the first screenshot

jensimmons’s picture

Oh, and we must fix the bug Jeff reported in #41-42. We need a search button in IE7!

(I just meant that it wasn't 'critical' according to the definition we were using on the Bartik queue, where critical meant it had to be fixed *before* Bartik to go into core. It's definitely a big deal and must be fixed.)

bleen’s picture

Status: Needs work » Needs review
FileSize
968 bytes

The problem is that the search form (correctly) has a different #ID in different contexts. This is correct because it can appear more than once on a page. There were a couple of styles that were aimed at the search block by #ID as it appears on the homepage and that caused the difference in styles.

bleen’s picture

FileSize
2.15 KB

This patch also fixes the search button in IE6 & 7

Its ugly css (for IE) but I couldn't make anything else work.

Status: Needs review » Needs work

The last submitted patch, bartik-search.patch, failed testing.

aspilicious’s picture

-  padding-right: 20px;
+  padding: 0 20px;

is unrelated and already in ;)

bleen’s picture

Status: Needs work » Needs review
FileSize
1.63 KB

last patch had some cruft

aspilicious’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
1.49 KB
1.72 KB

Your patch has a

+}
\ No newline at end of file

So I rolled a new one, I just rerolled with a extra newline.
This is rtbc, I got a screenshot to confirm ;)

bleen’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
2.11 KB

This is a slightly better patch because the search bar and button are more nicely centered in the block in IE7 with this patch

  • IE7:
    Windows 7 [Running]
  • IE6:
aspilicious’s picture

Status: Needs review » Reviewed & tested by the community

Confirmed

ff1’s picture

Confirmed. IE7 looks much better with the latest patch. Also tested in firefox.

bleen’s picture

bump

Jeff Burnz’s picture

Status: Reviewed & tested by the community » Needs work

In its current state the magnifying glass icon fails WCAG2 contrast requirements - afaikt its #737373 on a gradient background (top around #f2f2f2, bottom around #ececec - just the bit of the gradient that the icon sits over).

If the little magnifying glass is darkened to #666 or darker it passes.

Maybe this is a different issue, sorry if this is creep, maybe we can just nip this one here?

aspilicious’s picture

Status: Needs work » Reviewed & tested by the community

Going to put it back to rtbc.

#58 is another issue. Made a new one ==> #889660: Magnifying glass need to be darkened
Tried to edit the picture myself but it didn't work out as it hasn't have one solid color.
This patch is a bartik major issue, so I want to get it in as soon as possible.

Srry Jeff!

Jeff Burnz’s picture

Right on, lets get this in a move on, +1.

Jeff Burnz’s picture

#54: search-block.patch queued for re-testing.

Jeff Burnz’s picture

Status: Reviewed & tested by the community » Needs work
FileSize
3.24 KB
6.84 KB
23.87 KB

Setting back to needs work - there are a number of issues with search forms even with this patch. While the patch does solve the issue of the broken search submit button in IE7 after looking at the CSS more closely I can see most of the issues are actually caused by some of the original CSS. There also appears to be a rather verbose work-around for text-indent not working correctly in IE.

I have attached a screen-shot of the advanced search page - clearly we can see the search buttons are wrapping awkwardly and really the "Enter your keywords" label should probably be above the text input field.

The issue of the misplaced search button in the advanced search fieldset appears to be weight issue with the Languages criterion - they are priting below the search button so that is another issue.

bartik-search-form-issues.png

The search buttons are also rather lost on the background color - they are not clearly defined. Why not give them a border? In IE without a border they are almost invisible - I can only see the magnifying glass in some color schemes - and since we've lost text resizing to make it bigger I think we need more impact here - a border will lift the visibility of the button.

bartik-search-buttons.png

I've taken some time to write up another approach to theming these search form inputs - it uses floats and negates much of the special casing required for IE and the various elements.

Note that this patch solves a couple of issues that the patch in #54 does not:

1) fixes the display of the search form on the search results page (places the label above the text input, which automagically aligns the button correctly also)

2) adds more definition to the button by using a border

3) correctly sets the height on buttons for IE so they are the same height as the text input, and sets a proper width on the search button - before it was relying on inherited padding which is fragile.

4) removes a fair amount of cruft from the CSS, especially the IE stuff

@bleen18 - have i missed anything here - there was a bit of "width auto" in there - although it was not clear to me what the search block is supposed to look like in the #block-system-main (e.g. in the highlighted region).

Jeff Burnz’s picture

Status: Needs work » Needs review

bot...

bleen’s picture

Status: Needs review » Reviewed & tested by the community

I've been playing with #62 for the last hour and I think it covers everything from #54 and a bit more .... looks good to me and I think its high time we fix this sucker... RTBC

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.