Skip to:
Content

Opened 4 years ago

Closed 4 years ago

#5586 closed enhancement (fixed)

Add ID to editfield DIV

Reported by: sooskriszta Owned by: boonebgorges
Milestone: 2.1 Priority: normal
Severity: normal Version:
Component: Appearance - Template Pack Keywords: needs-patch good-first-bug
Cc: hnla, karmatosed, vivek@…

Description

All custom xprofile fields are provided wrapped in the editfield DIV. That's fantastic.

Sometimes, the site admins might want to manipulate the layout of specific fields...e.g. they may want 2 of the fields to sit in the same line horizontally instead of being in separate lines (above/below). Or they might want to right align a particular field for emphasis. There could be many other use cases.

Hence, I'd recommend that alongwith the editfield CLASS, the DIV should also have an ID which takes the field number value.

This way site admins can manipulate the look and field of individual fields via CSS.

Change History (14)

#1 @sooskriszta
4 years ago

  • Cc vivek@… added

#2 @boonebgorges
4 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 2.1

Good idea, and easy to implement.

#3 @DJPaul
4 years ago

  • Keywords reporter-feedback added
  • Milestone changed from 2.1 to Awaiting Review

While this is straightforward to patch, I'm not sure why. @sooskriszta, could you explain the use case further? The "editfield" divs have the field name as a class, e.g. field-1. I'm not sure why adding an ID will let you do anything that you can't already do using the existing classes.

#4 @boonebgorges
4 years ago

  • Keywords close added

DJPaul - You're right. I didn't look at the codebase before responding previously. It seems to me that the class is probably good enough. What do you think, sooskriszta?

#5 @sooskriszta
4 years ago

@DJPaul The fields themselves do have fieldname e.g. field-1 as id, but the divs don't have any ids. The div class is editfield. All field divs are of the class editfield. This works fine if I want to format ALL fields in the same way, but not if I want to apply specific formatting to specific fields. (I am trying this on BuddyPress 2.0 so if things have changed since then then I take all this back).

This is the output I am getting at the moment:

<div class="editfield">
<label for="field_14">Derék (cm)</label>
<input id="field_14" name="field_14" type="number" value=""/>
<p class="field-visibility-settings-notoggle" id="field-visibility-settings-toggle-14">
This field can be seen by:
<span class="current-visibility-level">Everyone</span>
</p>
<p class="description"/>
</div>

I want to change the <div class="editfield"> above to <div class="editfield" id="field_14">

The issue is at what level the identifier is inserted. e.g. my extension http://www.opencart.com/index.php?route=extension/extension/info&extension_id=6500 for OpenCart is decently popular because when people want to format categories, it makes a difference whether the ID is attached to the <li> or the content inside it.

e.g. I consider myself fairly proficient in CSS but I don't know how I would, in BuddyPress, put 2 specific fields in the same horizontal line short of ugly hacks which may not work well on all screens.

Last edited 4 years ago by sooskriszta (previous) (diff)

#6 @sooskriszta
4 years ago

  • Keywords reporter-feedback removed

#7 @hnla
4 years ago

Generally with a series of elements such as this, forms, lists etc things that repeat often in a loop sense it useful to ensure the main parent has a unique token, I'll nearly always drop a '-1', '-2' in the class list. adding a token to the parent is important - although we are well catered for here - as we tend to always hook rules via descendent selectors thus one works from the outermost parent which allows us to easily target the children, often seen is the error where parent has no token or tokens only on children which causes issues.

Adding a class 'editfield-1' ought to be sufficient along with existing class 'editfield' for generic styles.

Sidenotes: See underscores in those tokens ought really to be hyphens always in tokens. That div editfield would better be marked up as a 'fieldset' but perhaps that's a matter for template pack to pick up on.

#8 @sooskriszta
4 years ago

So we should change <div class="editfield"> to <div class="editfield field_14"> or <div class="editfield editfield-14"> (as opposed to <div class="editfield" id="field_14">)? I'm fine with that.

P.S. I agree underscores should be hyphens. Unfortunately, BuddyPress currently uses underscores...would be nice if that's changed in the template pack as well.

#9 @sooskriszta
4 years ago

  • Keywords close removed
  • Milestone changed from Awaiting Review to 2.1

Hope I'm not going beyond my remit by changing the milestone back to 2.1

#10 @DJPaul
4 years ago

  • Keywords reporter-feedback added

Interesting. With twentyfourteen + BP 2.0, I get this markup on the edit profile page:

<div class="editfield field_2 field_test alt field_type_textbox">

	<label for="field_2">test </label>
	<input id="field_2" name="field_2" type="text" value="">
	<p class="field-visibility-settings-toggle" id="field-visibility-settings-toggle-2">
	This field can be seen by: <span class="current-visibility-level">Everyone</span> <a href="#" class="visibility-toggle-link">Change</a>
	</p>


//and so on

@sooskriszta, what theme are you using? What exact URL/page are you seeing this on: the registration template/form?

Last edited 4 years ago by DJPaul (previous) (diff)

#11 @sooskriszta
4 years ago

I am using a non-BuddyPress theme (Weaver II Pro) so the template pack is being used. I see this on the registration form.

You can check it out at http://csillamvilag.com/modellek/regisztracio/ This is a fresh install.

#12 @boonebgorges
4 years ago

  • Keywords good-first-bug added

It looks like we are not using bp_field_css_class( 'editfield' ); in our /members/register/ templates, in the same way that we are in /members/single/profile/edit.php.

#13 @sooskriszta
4 years ago

  • Keywords reporter-feedback removed

If you decide to accept #5583 then we'd need to apply the fix in a different place (activation step).

#14 @boonebgorges
4 years ago

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

In 8416:

Add field-specific CSS classes to xprofile field divs in register.php

This is for parity with the members/single/profile/edit.php templates.

Fixes #5586

Note: See TracTickets for help on using tickets.