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.

Sign up for GitHub

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

Hotfix/gh404 lost center global flex issue #405

Merged
merged 3 commits into from
Mar 1, 2018
Merged

Hotfix/gh404 lost center global flex issue #405

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2efb0c1
[gh404] Adds failing tests for overriding global flexbox settings
peterramsing Feb 27, 2018
90c43c6
[gh404] Fixes issue where user couldn't override global flex setting …
peterramsing Feb 27, 2018
7dc19d7
[gh404] Removes un-needed whitespace
peterramsing Mar 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions lib/lost-center.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ module.exports = function lostCenterDecl(css, settings, result) {
lostCenterPadding = declArr[1];
}

if(lostCenterFlexbox !== 'flex') {
lostCenterFlexbox = settings.flexbox;
}

if (declArr.indexOf('flex') !== -1) {
lostCenterFlexbox = 'flex';
}
Expand All @@ -32,6 +36,8 @@ module.exports = function lostCenterDecl(css, settings, result) {
lostCenterFlexbox = 'no-flex';
}

lostCenterFlexbox = lgLogic.parseLostProperty(decl.parent.nodes, 'lost-center-flexbox', lostCenterFlexbox);

Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to leave this line blank? Wasn't sure it it was significant? If it is I'll shut up, otherwise it would neaten it up a very tiny bit :)

Copy link
Owner Author

@peterramsing peterramsing Feb 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

You're not the first person who's commented on my whitespace. I love it to a fault. I'll clean that up! Thanks for catching that!

lostCenterPadding = lgLogic.parseLostProperty(decl.parent.nodes, 'lost-center-padding', lostCenterPadding);
lostColumnRounder = lgLogic.parseLostProperty(decl.parent.nodes, 'lost-column-rounder', lostColumnRounder);

Expand All @@ -42,12 +48,6 @@ module.exports = function lostCenterDecl(css, settings, result) {
lostUnit = settings.gridUnit;
}

lostCenterFlexbox = lgLogic.parseLostProperty(decl.parent.nodes, 'lost-center-flexbox', lostCenterFlexbox);

if(lostCenterFlexbox !== 'flex') {
lostCenterFlexbox = settings.flexbox;
}

if(declArr[0] !== undefined && isFractionValue(declArr[0])) {
lostCenterMaxWidth = lgLogic.calcValue(declArr[0], lostColumnGutter, lostColumnRounder, lostUnit);
} else {
Expand Down
15 changes: 15 additions & 0 deletions test/lost-center.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,19 @@ describe('lost-center', function() {
'a { display: flex; flex-flow: row wrap; max-width: 1140px; margin-left: auto; margin-right: auto; padding-left: 30px; padding-right: 30px }'
);
});

it('overrides global flex when told to', function() {
check(
'@lost flexbox flex; .root { lost-center: 600px no-flex; }',
'.root { max-width: 600px; margin-left: auto; margin-right: auto; }\n' +
'.root:before { content: \'\'; display: table; }\n' +
'.root:after { content: \'\'; display: table; clear: both; }'
);
check(
'@lost flexbox flex; .root { lost-center: 600px; lost-center-flexbox: no-flex;}',
'.root { max-width: 600px; margin-left: auto; margin-right: auto; }\n' +
'.root:before { content: \'\'; display: table; }\n' +
'.root:after { content: \'\'; display: table; clear: both; }'
);
});
});