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

Hidden columns #82

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

Falieson
Copy link

  • updated docs
  • updated tests
  • updated Col.js component with hiddenMap var and if/else instead of ternary

resolved issues

Copy link
Collaborator

@silvenon silvenon left a comment

Choose a reason for hiding this comment

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

Thanks for this, I love it! Would you mind rebasing and addressing the comment below? I realize a lot of time has passed, if you're not in the mood, I can take it from here, no problem. 😉

@@ -44,7 +44,7 @@
"expect": "^1.13.0",
"express": "^4.13.3",
"extract-text-webpack-plugin": "^0.8.2",
"flexboxgrid": "^6.3.0",
"flexboxgrid": "Zetoff/flexboxgrid#hidden-columns",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this temporary?

Copy link
Author

Choose a reason for hiding this comment

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

Hi @silvenon yeah: this is temporary because it depends on this flexboxgrid branch kristoferjoseph/flexboxgrid#211

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, I see! Maybe this PR should wait then until there are news about that feature.

Copy link
Author

Choose a reason for hiding this comment

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

I think that'd be fine - hopefully @kristoferjoseph merges that PR soon

@Falieson
Copy link
Author

Falieson commented May 2, 2017

@silvenon there's finally been some movement on that flexboxgrid PR. The author of FBG wants to keep the package size small, and if people want the extra features use https://github.com/lgg/flexboxgrid-helpers

I will resubmit this PR with the extra pkg dependency, or if you prefer - I can maintain a fork of FBG with extended features kristoferjoseph/flexboxgrid#211 (comment)

@cyberhck
Copy link

cyberhck commented Aug 2, 2017

@Falieson any news on this?

@mattotodd
Copy link

+1

@Falieson
Copy link
Author

Falieson commented Aug 8, 2017

Sorry - I'm swamped with my new job. This react-flexbox-grid package should be updated to use https://github.com/evgenyrodionov/flexboxgrid2 , I am very glad that @evgenyrodionov (author of redux-logger) stepped up to maintain the package as I'm obviously not the best choice to be the new maintainer. I think that @evgenyrodionov will adopt hidden-columns as part of the new flexboxgrid2 package rather than requiring a helper package.
Whether or not this branch includes the package change is up to you, but this PR still has to be done on the new package: kristoferjoseph/flexboxgrid#211

@benoj
Copy link

benoj commented Aug 22, 2017

Anyone taking over this PR?

@anton6
Copy link

anton6 commented Apr 7, 2018

Should this one be marked as closed in favour of #138?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants