Skip to content

Fix #805#806

Merged
acicovic merged 2 commits intoatk4:developfrom
karakal:patch-1
Oct 2, 2019
Merged

Fix #805#806
acicovic merged 2 commits intoatk4:developfrom
karakal:patch-1

Conversation

@karakal
Copy link
Copy Markdown
Contributor

@karakal karakal commented Sep 27, 2019

#Fix of Error in addFilterColumn (see ticket #805 ).

@codecov
Copy link
Copy Markdown

codecov bot commented Sep 27, 2019

Codecov Report

Merging #806 into develop will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop     #806      +/-   ##
=============================================
+ Coverage      76.23%   76.28%   +0.04%     
- Complexity      2291     2293       +2     
=============================================
  Files            119      119              
  Lines           5404     5414      +10     
=============================================
+ Hits            4120     4130      +10     
  Misses          1284     1284
Impacted Files Coverage Δ Complexity Δ
src/FormField/Input.php 100% <0%> (ø) 32% <0%> (+2%) ⬆️
src/FormField/DropDown.php 92.68% <0%> (+0.09%) 39% <0%> (ø) ⬇️
src/FormField/Lookup.php 76.5% <0%> (+0.12%) 56% <0%> (ø) ⬇️
src/FormField/AutoComplete.php 66.31% <0%> (+0.35%) 33% <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 dae88ae...5d61d81. Read the comment docs.

@acicovic acicovic self-requested a review October 1, 2019 18:22
@acicovic acicovic requested a review from abbadon1334 October 1, 2019 18:25
to pass check
Copy link
Copy Markdown
Collaborator

@abbadon1334 abbadon1334 left a comment

Choose a reason for hiding this comment

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

@acicovic di you test it? i don't, but the issue is clear
LGTM ( i remove the extra line to pass the tests )
@karakal thanks!

@acicovic
Copy link
Copy Markdown
Collaborator

acicovic commented Oct 2, 2019

@abbadon1334 that was my thought also, but after testing I see that it doesn't behave exactly like TypeString as the column expands at maximum width. So the exception is indeed fixed but there should be an additional fix to make it work like string. I suspect the factoryType() function within FilterModel\Generic. Will take a look at it if I have time. @karakal if you have the time to spot and fix this, go ahead.

@acicovic
Copy link
Copy Markdown
Collaborator

acicovic commented Oct 2, 2019

OK, so this is by design. ATK assigns an atk-cell-expanded class in TableColumn\Text.php. This is what makes it look different, and for a text field this is logical. So merging this.

@acicovic acicovic merged commit 45c5130 into atk4:develop Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants