-
Notifications
You must be signed in to change notification settings - Fork 2
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
[PF-17] - Layout imports rows cols with default value 0 #18
Conversation
Release 1.3.0
- set default layout type to legacy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the evaluation of the rows
property work as intended?
src/lib/import/import-utils.ts
Outdated
dataGroup.cols = cols; | ||
} | ||
const rows = this.parseNumberValue(xmlDataGroup, 'rows'); | ||
if (rows && rows >= 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if rows is 0 rows && rows >= 0
evaluates to false
since 0 is a falsy value
src/lib/import/import.service.ts
Outdated
trans.layout.cols = cols; | ||
} | ||
const rows = this.importUtils.parseNumberValue(xmlLayout.item(0), 'rows'); | ||
if (rows && rows >= 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if rows is 0 rows && rows >= 0 evaluates to false since 0 is a falsy value
- fix condition for 0 value
- fix condition for 0 value
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
Legacy layout should be the default value. Layout rows and cols should be imported only if layout type is defined and is not Legacy. Rows can be undefined or 0+, cols can be undefined or 1+.
Fixes PF-17