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

Fix Bugs 05/04/2024 #15

Merged
merged 11 commits into from
Apr 5, 2024
Merged

Fix Bugs 05/04/2024 #15

merged 11 commits into from
Apr 5, 2024

Conversation

Barny-Thorpe
Copy link
Contributor

@Barny-Thorpe Barny-Thorpe commented Apr 5, 2024

Fix Current Bugs 05/04/2024

Not Checking the total selector field

The section of JS checking if the field was the correct input was removed.

  • This has been added back now so only the specified field is chosen.
  • Uses the 'closest' method in js to select itself or any of its ancestors - this is necessary as the ginput-amount class is found on the input itself whereas the field ids are located on the outermost gfield div.

Values displaying Nan after number input adds pound sign

When clicking off the input the number field automatically adds the pound sign.
This cant be converted to a float so returns a NaN value - as this is still technically not empty it passes this through to the values.

  • Earlier empty checking and regex string sanitisation has been implemented to prevent this.

image

Chosen field not being saved in the dropdown in editor

The field builder currently doesn't show the saved value of the chosen field in the dropdown.

gift-aid-not-saving.mp4

@Barny-Thorpe Barny-Thorpe marked this pull request as ready for review April 5, 2024 10:58
@Barny-Thorpe Barny-Thorpe requested a review from a team April 5, 2024 11:01
Comment on lines +15 to +24
window.gform.addAction(
'gform_input_change',
function (elem) {
if (
!(elem instanceof HTMLInputElement) ||
!elem.closest(totalSelector) ||
!elem.value
) {
return;
}
Copy link
Member

Choose a reason for hiding this comment

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

what theeeee

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is what the formatter wanted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just leaves in the breeze,
drifting onwards, no control,
formatter decides

Copy link
Member

Choose a reason for hiding this comment

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

eww

Comment on lines +37 to +40
$exists = GFAPI::form_id_exists($form_id);
if (empty($exists)) {
return;
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
$exists = GFAPI::form_id_exists($form_id);
if (empty($exists)) {
return;
}
if (! GFAPI::form_id_exists($form_id)) {
return;
}

stop doing empty checks on boolssssss!!!!!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

theres literally no harm in doing it - it just adds an extra layer of safety in case the function doesnt do what its docblock says

Comment on lines +57 to +58
<option value="">--Please select a field--</option>

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<option value="">--Please select a field--</option>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no

src/GfGiftAidField.php Outdated Show resolved Hide resolved
src/GfGiftAidField.php Outdated Show resolved Hide resolved
src/GfGiftAidField.php Outdated Show resolved Hide resolved
Copy link
Member

@codepuncher codepuncher left a comment

Choose a reason for hiding this comment

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

const > var is only required thing

@Barny-Thorpe Barny-Thorpe merged commit 88562ed into main Apr 5, 2024
2 checks passed
@Barny-Thorpe Barny-Thorpe deleted the fix-nan-and-save-bugs branch April 5, 2024 15:01
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.

None yet

2 participants