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

4.x - Automatically generate placeholders for floating labels. #367

Merged
merged 1 commit into from
Apr 4, 2022

Conversation

ndm2
Copy link
Collaborator

@ndm2 ndm2 commented Apr 4, 2022

While this will not cover custom types, it should be good enough for most scenarios, eg use of text and textarea types.

The text inference is copied from the core, but without support for the special _ids key, as this would by default cause a select input to be generated, where placeholders do not apply.

Refs #349 (comment) /cc @gerhard-lechner

@codecov
Copy link

codecov bot commented Apr 4, 2022

Codecov Report

Merging #367 (ca92f1a) into master (81910e1) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #367      +/-   ##
============================================
+ Coverage     99.22%   99.23%   +0.01%     
- Complexity      360      368       +8     
============================================
  Files            21       21              
  Lines          1027     1043      +16     
============================================
+ Hits           1019     1035      +16     
  Misses            8        8              
Impacted Files Coverage Δ
src/View/Helper/FormHelper.php 99.13% <100.00%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 81910e1...ca92f1a. Read the comment docs.

* Modify options for placeholders.
*
* @param string $fieldName Field name.
* @param array $options Options. See `$options` argument of `control()` method.
Copy link
Member

Choose a reason for hiding this comment

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

we usually array<string, mixed> those
Same for assoc return value.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd say that's something for a new PR if we want to adpot that syntax, as there's dozens of places that just use the plain array hints.

Copy link
Member

Choose a reason for hiding this comment

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

Well, new code could already do that, the same as you just added return types :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The other option processing methods already have return types, I only copy pasta'd it 😉

@dereuromark dereuromark merged commit 6924ce4 into FriendsOfCake:master Apr 4, 2022
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

3 participants