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

Feature/fix textarea #783

Merged
merged 10 commits into from
Aug 4, 2019
Merged

Feature/fix textarea #783

merged 10 commits into from
Aug 4, 2019

Conversation

DarkSide666
Copy link
Member

@DarkSide666 DarkSide666 commented Jul 23, 2019

Related: #755, #760, #781

Needs testing and try to reproduce HTML markup bug we faced before. I couldn't find how to reproduce that.

Tests are in /demos/form.php

Copy link
Collaborator

@PhilippGrashoff PhilippGrashoff left a comment

Choose a reason for hiding this comment

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

Looks way cleaner than code before!

Copy link
Member

@romaninsh romaninsh left a comment

Choose a reason for hiding this comment

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

Use this test template and add unit tests:

#776

But yes - looks cleaner!

[edit: linked to incorrect file, it's FormTest.php]

@romaninsh
Copy link
Member

nevermind i'll add test..

// dropdown insists for value to be there
$this->assertFieldError('opt3', 'Must not be null');

// value with '0' is valid selection
Copy link
Member

Choose a reason for hiding this comment

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

@DarkSide666 - here is a test for dropdown value with key '0' which currently is considered invalid incorrect selection.

@romaninsh
Copy link
Member

romaninsh commented Jul 24, 2019

@DarkSide666 i have added a test-suite, but it still not working with 'values'=>['0'=>'Female'] , so

not fixing #781

@acicovic
Copy link
Collaborator

Unfortunately the HTML markup bug (#760) reoccurs for me with this code. @romaninsh is everything OK on your side?

@romaninsh
Copy link
Member

@acicovic - i haven't tested against the project actually, have been a bit busy. @DarkSide666 can you see if saasty admin works OK (it uses mastercrud heavily)

@romaninsh romaninsh added the hangout agenda 🔈 Will discuss on next hangout label Jul 29, 2019
@codecov
Copy link

codecov bot commented Aug 2, 2019

Codecov Report

Merging #783 into develop will increase coverage by 0.11%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             develop    #783      +/-   ##
============================================
+ Coverage      71.18%   71.3%   +0.11%     
+ Complexity      2246    2242       -4     
============================================
  Files            119     119              
  Lines           5328    5325       -3     
============================================
+ Hits            3793    3797       +4     
+ Misses          1535    1528       -7
Impacted Files Coverage Δ Complexity Δ
src/Form.php 78.08% <ø> (+2.05%) 68 <0> (ø) ⬇️
src/FormField/TextArea.php 100% <100%> (ø) 3 <0> (-2) ⬇️
src/FormField/DropDown.php 90.12% <100%> (+2.02%) 39 <0> (-2) ⬇️
src/TableColumn/Status.php 82.85% <0%> (+5.71%) 12% <0%> (ø) ⬇️

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 0ed93f2...322f797. Read the comment docs.

@codecov
Copy link

codecov bot commented Aug 2, 2019

Codecov Report

Merging #783 into develop will increase coverage by 0.09%.
The diff coverage is 100%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop     #783      +/-   ##
=============================================
+ Coverage      71.17%   71.26%   +0.09%     
+ Complexity      2246     2242       -4     
=============================================
  Files            119      119              
  Lines           5328     5325       -3     
=============================================
+ Hits            3792     3795       +3     
+ Misses          1536     1530       -6
Impacted Files Coverage Δ Complexity Δ
src/Form.php 78.08% <ø> (+2.05%) 68 <0> (ø) ⬇️
src/FormField/TextArea.php 100% <100%> (ø) 3 <0> (-2) ⬇️
src/FormField/DropDown.php 90.12% <100%> (+2.02%) 39 <0> (-2) ⬇️
src/TableColumn/Status.php 82.85% <0%> (+2.85%) 12% <0%> (ø) ⬇️

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 ac79ff6...e322a88. Read the comment docs.

@acicovic
Copy link
Collaborator

acicovic commented Aug 3, 2019

For me this resolves #755 and #760. Doesn't fix #781 but this is not related to textarea per se, so I propose to merge this and we can tackle #781 afterwards.

@DarkSide666
Copy link
Member Author

fix #755
fix #760

@DarkSide666 DarkSide666 merged commit 56e6843 into develop Aug 4, 2019
@DarkSide666 DarkSide666 deleted the feature/fix-textarea branch August 4, 2019 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hangout agenda 🔈 Will discuss on next hangout help wanted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants