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

Use first label to avoid smushing nested labels in more complex fields #126

Merged
merged 1 commit into from
Dec 8, 2016

Conversation

hackartisan
Copy link
Contributor

@hackartisan hackartisan commented Dec 8, 2016

If you've created more complex fields for, e.g., nested objects the "Add additional" link includes every label all mushed together. This is the fix.

@projecthydra/hyrax-code-reviewers

@jcoyne
Copy link
Member

jcoyne commented Dec 8, 2016

Can you show a picture of the DOM before and after this fix?

@hackartisan
Copy link
Contributor Author

before:
screen shot 2016-12-08 at 2 25 29 pm

after:
screen shot 2016-12-08 at 2 32 43 pm

@@ -122,7 +122,7 @@ export class FieldManager {

getFieldLabel($element, options) {
var label = '';
var $label = $element.find("label");
var $label = $element.find("label").first();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HackMasterA does .find always return an array, even if there's only one item?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not really an array; it's a jquery object. but good question. it does always work with .first() and this doesn't mess up any of my other fields. i can add in a test case to demonstrate if you want although you just merged so that's sort of confusing :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HackMasterA you're test appeared to cover the scenario I was questioning, so I think we're good.

@awead awead merged commit c1e9d29 into master Dec 8, 2016
@awead awead deleted the first-label branch December 8, 2016 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants