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

More buttonrama fallout #18820

Merged
merged 1 commit into from
Oct 22, 2020
Merged

More buttonrama fallout #18820

merged 1 commit into from
Oct 22, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Fixes a notice from Buttonrama - affects 5.31+

Before

Screen Shot 2020-10-21 at 6 25 18 PM

After

poof

Technical Details

Comments

@civibot civibot bot added the 5.31 label Oct 21, 2020
@civibot
Copy link

civibot bot commented Oct 21, 2020

(Standard links)

@demeritcowboy
Copy link
Contributor

demeritcowboy commented Oct 21, 2020

I don't think this is the same as with static. This seems like an instance where it should be passing an array but isn't:

$this->add('File', 'uploadFile', ts('Import Data File'), 'size=30 maxlength=255', TRUE);

And in fact here I would just pass NULL instead since size and maxlength don't mean much for file upload fields.

@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy Ah - ok - I assumed but yeah - let's fix to pass an array

@eileenmcnaughton
Copy link
Contributor Author

Fixed

@demeritcowboy
Copy link
Contributor

There's a double-space that snuck in there but otherwise looks good. Could also pass NULL since those html params don't mean much for file uploads. It didn't seem to make a difference when I tested.

@eileenmcnaughton eileenmcnaughton force-pushed the 531 branch 2 times, most recently from 73c98e0 to 91a0764 Compare October 21, 2020 20:27
@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy OK - I went with NULL

@demeritcowboy
Copy link
Contributor

Looks good!

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

Successfully merging this pull request may close these issues.

3 participants