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(stark-ui): cellFormatter is now called even when the rawValue is undefined #1468

Conversation

SuperITMan
Copy link
Member

ISSUES CLOSED: #1465

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #1465

What is the new behavior?

cellFormatter is now triggered even if the rawValue is undefined.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@SuperITMan SuperITMan added this to the 10.0.0-rc.3 milestone Dec 18, 2019
@SuperITMan SuperITMan added this to In progress in 10.0.0-rc.3 via automation Dec 18, 2019
@coveralls
Copy link

coveralls commented Dec 18, 2019

Coverage Status

Coverage increased (+0.02%) to 94.402% when pulling 815ac6c on SuperITMan:bugfix/table-cellformatter-not-triggered into 0d6e28f on NationalBankBelgium:master.

@christophercr christophercr changed the title fix(stark-ui): fixed column issue with cellFormatter which is not called when the rawValue is undefined fix(stark-ui): cellFormatter is now called even when the rawValue is undefined Dec 18, 2019
Copy link
Collaborator

@christophercr christophercr left a comment

Choose a reason for hiding this comment

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

Small remarks

component.ngAfterViewInit();
});

it("should display 'ONE' instead of '1' when id == '1'", () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's put in the test description only the functionality tested instead of the specific details... for example: should display the formatted value in the cell instead of the raw value

expect(rowIdElements[0].innerText).toEqual("one");
});

it("should display '-null-' when 'description' is undefined", () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

same remark about the test description, what about: should display the formatted value in the cell even if the raw value is undefined ?

expect(rowIdElements[1].innerText).toEqual("-null-");
});

it("should NOT display anything when 'test' is undefined and no 'cellFormatter' property is set for the column", () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

same remark about the test description, what about: should NOT display anything when the raw value is undefined and there is no 'cellFormatter' defined for the column ?

@SuperITMan SuperITMan force-pushed the bugfix/table-cellformatter-not-triggered branch 2 times, most recently from cae23cb to 397bec8 Compare December 18, 2019 14:54
@SuperITMan
Copy link
Member Author

PR updated. @christophercr @nicanac Could you please check it ? 😊

Copy link
Collaborator

@christophercr christophercr left a comment

Choose a reason for hiding this comment

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

LGTM

10.0.0-rc.3 automation moved this from In progress to Reviewer approved Dec 18, 2019
@SuperITMan SuperITMan force-pushed the bugfix/table-cellformatter-not-triggered branch from 397bec8 to 815ac6c Compare January 6, 2020 09:57
Copy link
Contributor

@nicanac nicanac left a comment

Choose a reason for hiding this comment

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

LGTM, 👍

@nicanac nicanac merged commit 52ae6e0 into NationalBankBelgium:master Jan 7, 2020
10.0.0-rc.3 automation moved this from Reviewer approved to Done Jan 7, 2020
@SuperITMan SuperITMan deleted the bugfix/table-cellformatter-not-triggered branch January 20, 2020 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
10.0.0-rc.3
  
Done
Development

Successfully merging this pull request may close these issues.

ui: table - cellFormatter property is not called when the cell does not contain a value
4 participants