WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#7678 closed defect (bug) (fixed)

Front Page with 2 galleries doesn't respect settings for each

Reported by: MtDewVirus Owned by:
Milestone: 2.8 Priority: normal
Severity: normal Version: 2.7
Component: Gallery Keywords: has-patch tested commit
Focuses: Cc:

Description

If your front page has two posts with galleries and each gallery is set to display a different number of columns, the CSS from the oldest (lowest displayed post on the page) is used.

Publish a post with a gallery and set columns="2" and then publish another post without setting the columns. The CSS for each gallery is in the HTML, but both posts will use the CSS to display 2 columns.

Tested on Firefox 3.0.1 Mac and r8797.

Attachments (4)

7678.diff (1006 bytes) - added by jamescollins 5 years ago.
patch against r11091
7678_alternative.diff (1.2 KB) - added by jamescollins 5 years ago.
Alternative patch using static
7678_combined.diff (1.3 KB) - added by jamescollins 5 years ago.
uses unique css id for styling, and a galleryid-x class
7678_combined2.diff (1.6 KB) - added by jamescollins 5 years ago.

Download all attachments as: .zip

Change History (25)

comment:1 DD325 years ago

  • Component changed from General to Gallery
  • Keywords needs-patch added
  • Owner anonymous deleted

Media is getting an overhaul in 2.8, This'll probably be skiped for the 2.7 release.

comment:2 ShaneF5 years ago

  • Milestone changed from 2.7 to 2.8

comment:3 jamescollins5 years ago

This is also a problem on a WP page that contains multiple [gallery] shortcodes with different columns=x parameters.

The CSS rules from the last gallery override all the other galleries.

This problem is also discussed <a href="http://en.forums.wordpress.com/topic/problem-with-gallery-columns">in the WP.com forums</a>.

jamescollins5 years ago

patch against r11091

comment:4 jamescollins5 years ago

  • Keywords has-patch added; needs-patch removed

Simple patch (7678.diff) adds a unique id to each gallery (based on the post id).

The CSS rules ouptutted with each gallery are then specific to that gallery id, and shouldn't affect any other galleries on the same page.

comment:5 follow-up: DD325 years ago

@jamescollins: Nice idea, And its backwards compatible too!

comment:6 in reply to: ↑ 5 ; follow-up: jamescollins5 years ago

Replying to DD32:

@jamescollins: Nice idea, And its backwards compatible too!

Thanks DD32.

The only problem I can think of would be that if you have the multiple galleries for the same page/post id on a single page.

For example, you have page (id 5) which has a lot of text, with two gallery shortcodes on it:

[gallery columns=1]

text here.

[gallery columns=4]

In this case, the CSS class for both galleries would be set to galleryid-5, so the same problem would still occur (both galleries would be displayed with 4 columns)

The other option could be to use a static counter variable that is incremented by one every time the [gallery] shortcode is parsed. This guarantees a unique class selector for each gallery on each page.

jamescollins5 years ago

Alternative patch using static

comment:7 in reply to: ↑ 6 jamescollins5 years ago

Replying to jamescollins:

The other option could be to use a static counter variable that is incremented by one every time the [gallery] shortcode is parsed. This guarantees a unique class selector for each gallery on each page.

Patch for this attached (see 7678_alternative.diff).

Which solution do you think is better?

comment:8 follow-up: DD325 years ago

Static var is better, Mind you, Its rare to have 2 galleries with the same postID visible at once.. (esp since it doesnt work right now)

comment:9 in reply to: ↑ 8 jamescollins5 years ago

Replying to DD32:

Static var is better, Mind you, Its rare to have 2 galleries with the same postID visible at once.. (esp since it doesnt work right now)

The first solution semantically cleaner, but the static var solution is more bullet proof.

What are the chances of getting this fixed for 2.8?

Thanks.

comment:10 follow-ups: DD325 years ago

  • Keywords dev-feedback added; gallery css removed

The first solution semantically cleaner, but the static var solution is more bullet proof.

Indeed.

Theres no real reason why we cant insert both classes into the HTML, But style based off the static version.. :)

Some dev feedback on commit or punt would be good.

comment:11 in reply to: ↑ 10 jamescollins5 years ago

Replying to DD32:

Theres no real reason why we cant insert both classes into the HTML, But style based off the static version.. :)

That's a good idea. Adding both the galleryid-x class and the gallery-y classes, but styling the gallery using the (more unique) gallery-y class sounds good to me.

comment:12 Denis-de-Bernardy5 years ago

more or less the same fix as #9400

comment:13 hakre5 years ago

I suggest to close #9400 (there) as a duplicate because the suggestion in #7678 (gere) are better.

comment:14 in reply to: ↑ 10 jamescollins5 years ago

Replying to DD32:

Theres no real reason why we cant insert both classes into the HTML, But style based off the static version.. :)

Some dev feedback on commit or punt would be good.

I've created 7678_combined.diff. It creates a unique gallery-x class (by using a static variable) which is used for styling the gallery, as well as the galleryid-y class which contains the post id of the gallery.

jamescollins5 years ago

uses unique css id for styling, and a galleryid-x class

comment:15 follow-up: Denis-de-Bernardy5 years ago

$id should be $post->ID, else you've the ID of the last attachment instead of that of the post. once that's updated, but +1 tested commit and please close #9400 as dup.

jamescollins5 years ago

comment:16 in reply to: ↑ 15 jamescollins5 years ago

  • Keywords dev-feedback removed

Replying to Denis-de-Bernardy:

$id should be $post->ID, else you've the ID of the last attachment instead of that of the post. once that's updated, but +1 tested commit and please close #9400 as dup.

Thanks Denis.

In 7678_combined2.diff I have changed the attachment loop to use $attid instead of $id, so it doesn't conflict with the $id gallery parameter.

comment:17 follow-up: Denis-de-Bernardy5 years ago

  • Keywords tested commit added

comment:18 in reply to: ↑ 17 jamescollins5 years ago

Replying to Denis-de-Bernardy:

Thanks for testing Denis. Is there any chance of getting this included in WP 2.8?

Thanks.

comment:19 Denis-de-Bernardy5 years ago

I'm pretty sure Ryan will add it. He and Andrew are going through the commit list, and the patch is totally harmless.

comment:20 Denis-de-Bernardy5 years ago

see also #8181 or one of the many related tickets, which may be worth looking into at one point.

comment:21 azaozz5 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [11339]) Support more than one gallery on the same page, props jamescollins, fixes #7678

Note: See TracTickets for help on using tickets.