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

Refactored includes/forms/template.php to have unique IDs #380

Closed
wants to merge 2 commits into from

Conversation

mathetos
Copy link
Member

Fixes #379

Also cleaned up markup to be more semantic. Required changes to JS files as well, so careful review would be necessary.

…board. Also cleaned up markup to be more semantic. Required changes to JS files as well, so careful review would be necessary.
@mathetos mathetos added the a11y label Oct 30, 2015
@DevinWalker
Copy link
Member

It's looking really good! I think the one thing that needs to be standardized is the underscore vs hyphen usage within the selectors. I think we should stick with the hypens as I a rarely see underscores used for classes/ids. Sound good? :)

@mathetos
Copy link
Member Author

@DevinWalker The PR is refreshed to make all IDs and Classes consistently use dashes instead of underscores. The "show terms" reveal link was also not working correctly previously, that's corrected in the refreshed PR.

@DevinWalker
Copy link
Member

@mathetos I also see you update the name attribute with hyphens which can cause issues with the way donations are processed. Let's discuss this PR together tomorrow.

<input class="give-input required" type="text" name="give_first" placeholder="<?php _e( 'First name', 'give' ); ?>" id="give-first" value="<?php echo is_user_logged_in() ? $user_data->first_name : ''; ?>"<?php if ( give_field_is_required( 'give_first', $form_id ) ) { echo ' required '; } ?>/>
</p>
<input class="give-input required" type="text" name="give-first" placeholder="<?php _e( 'First name', 'give' ); ?>" id="give-first-<?php echo $form_id; ?>" value="<?php echo is_user_logged_in() ? $user_data->first_name : ''; ?>"<?php if ( give_field_is_required( 'give-first', $form_id ) ) { echo ' required '; } ?>/>
</div>
Copy link
Member

Choose a reason for hiding this comment

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

It's OK for name attributes to use underscores

@DevinWalker
Copy link
Member

Closing this since it's out of date and incompatible now.

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.

2 participants