Skip to content
This repository has been archived by the owner on Nov 7, 2019. It is now read-only.

Pm 2019 02 issues #606

Merged
merged 8 commits into from
Mar 4, 2019
Merged

Pm 2019 02 issues #606

merged 8 commits into from
Mar 4, 2019

Conversation

philli-m
Copy link
Contributor

@philli-m philli-m commented Mar 4, 2019

fixes #443
fixes #593
fixes #428
might have fixed following #599

Copy link
Contributor

@fuzzylogic2000 fuzzylogic2000 left a comment

Choose a reason for hiding this comment

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

Nice!
I am just a tiny bit confused about the additions to package.json. If they need to stay, the ^ has to be removed.
Also the spacer: is that rem or em? And if it's em, can we maybe make it rem on bet.in?

package.json Outdated
@@ -27,6 +27,7 @@
"feature-detect": "1.0.0",
"file-loader": "3.0.1",
"file-saver": "1.3.8",
"i": "^0.3.6",
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this for?

package.json Outdated
@@ -36,6 +37,7 @@
"mapbox-gl-leaflet": "0.0.3",
"mini-css-extract-plugin": "0.5.0",
"node-sass": "4.11.0",
"npm": "^6.8.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

do we normally have npm in the requirements? I thought not?

@philli-m
Copy link
Contributor Author

philli-m commented Mar 4, 2019

@fuzzylogic2000 we have $padding which is 1rem and $spacer which is 1em shall we leave them different? I have updated where I used it to $padding as should have been on first one anyway

Copy link
Contributor

@fuzzylogic2000 fuzzylogic2000 left a comment

Choose a reason for hiding this comment

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

Yeah!

@fuzzylogic2000
Copy link
Contributor

@fuzzylogic2000 we have $padding which is 1rem and $spacer which is 1em shall we leave them different? I have updated where I used it to $padding as should have been on first one anyway

@phillimorland Yes, we can leave it here like that. But for later, I thought, we could change it? I think, we added the r-spacer in mB, because we thought, there was no real use in a spacer in em, but were scared to replace it in the much-used mB. Here I am not that scared. I think, we would notice if it breaks anything (and could even chcek, where it's used and if that in turn is used in bet.in) and there are not too many people using it.

@fuzzylogic2000 fuzzylogic2000 merged commit 1889927 into master Mar 4, 2019
@fuzzylogic2000 fuzzylogic2000 deleted the pm-2019-02-issues branch March 4, 2019 16:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants