Skip to content
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

Add unique class to price field template #21484

Merged
merged 1 commit into from
Sep 23, 2021

Conversation

JKingsnorth
Copy link
Contributor

@JKingsnorth JKingsnorth commented Sep 15, 2021

Motivation

Currently, classes are assigned to price fields based on the 'name' of the price field, eg:

image

However:

  • this is clunky to reference in CSS
  • when a price set is 'copied' - the price fields are copied along with it, and they keep the same 'name' as the original, eg:

image

... this can have the unintended consequence of customisations to one price field being cloned, and applying to the clone as well!

Solution

We don't want to change the current behaviour, as it might break peoples' templates and customisations.

However, let's add a new class to price fields, which includes the ID of the price field. This will make it easier to target them in CSS, and give them truly 'unique' names.

See the screenshots above, you can see the new classes 'crm-price-field-id-xxxx'

Technical Details

Minor template change to add a new css class in what seemed like a consistent format.

@civibot
Copy link

civibot bot commented Sep 15, 2021

(Standard links)

@civibot civibot bot added the master label Sep 15, 2021
@eileenmcnaughton
Copy link
Contributor

@colemanw do we do this or try to add as 'data'

@seamuslee001
Copy link
Contributor

I think this makes more sense to me especially if the goal is to do some css / theming changes but would appreciate @colemanw's thoughts as well

@JKingsnorth
Copy link
Contributor Author

JKingsnorth commented Sep 16, 2021

Thanks @eileenmcnaughton / @seamuslee001

The reason I went for this approach is that we have the crm-contribution-page-id-xxxx, crm-event-id-xxxx classes on the main containers for contribution and event pages (which is invaluable!). So I was trying to keep a consistent approach.

Also, I think it is useful to have these things easily accessible as classes for theming. eg: in drupal you have page-node-xxxx as a class.

To me, using the id in the class is a lot better than using the 'name' field - but I didn't want to remove the name-based CSS class and break everyone's stuff [=

@mattwire mattwire merged commit 75b52d0 into civicrm:master Sep 23, 2021
@mattwire
Copy link
Contributor

This will certainly help with custom theming

@JKingsnorth
Copy link
Contributor Author

Thanks @mattwire !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants