Importing withResponsiveness() helper #144
Labels
Comments
|
Hi @dupski, thanks for opening this issue! Well, we weren't planning to include that with the library, that's why is so hard to access. That said, this is not the first time that somebody asks for it. Should we include it in our dist folder then? |
|
Since is an option we have for the charts and is a feature listed in the docs I think it should be included or at least well referenced in the docs, but first I think we should fix the issue #105 that is still happening in |
This was referenced Dec 5, 2018
miglesiasEB
pushed a commit
that referenced
this issue
Dec 6, 2018
<!--- Provide a general summary of your changes in the Title above --> ## Description <!--- Describe your changes in detail --> The resizeMainHorizontal was added in the PR #84 as a hacking fix for the responsive container in the project documentation. This function broke the charts responsiveness outside the docs because it was looking for a `main` tag present in the **React Styleguidist** docs but not in every other project. This PR removes the problematic function and uses the correct one to fix the problem. The doc's responsiveness was tested using the original function and it work ok with this version of React Styleguidist. This PR also address the issue #144 to make it easier to import the ResponsiveContainer component and the withResponsiveness helper. ## Motivation and Context <!--- Why is this change required? What problem does it solve? --> <!--- If it fixes an open issue, please link to the issue here. --> * The charts' responsiveness is currently broken as described in the issue #105. * At the moment is difficult to import the ResponsiveContainer component and the withResponsiveness helper because you have to do it directly from the ESM bundle. ## How Has This Been Tested? <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> * All the unit tests were ran and all of them passed. * The documentation pages were reviewed to check for any inconsistency with the published one. ## Screenshots (if appropriate): <img width="1676" alt="wide" src="https://user-images.githubusercontent.com/282903/49551467-5007dc80-f8bd-11e8-9611-53de3d8166c9.png"> <img width="1237" alt="narrow" src="https://user-images.githubusercontent.com/282903/49551475-58601780-f8bd-11e8-9fd6-eaf84ecaf96e.png"> ## Types of changes <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] Refactor (changes the way we code something without changing its functionality) - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Checklist: <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have read the **CONTRIBUTING** document. - [ ] I have updated the documentation accordingly. - [ ] I have added tests to cover my changes. - [x] All new and existing tests passed.
|
All ready and working properly! |
|
@miglesiasEB Hi guys, sorry to bother you. I was wondering if you have a date to upload this to npm. I'm having some issue with this on a typescript project and jest.... and I ran out of ideas to make it work for the tests :c |
|
Hi @juanpaucar, I just released 0.5.1 with this fix, it should be ready in NPM soon! |
|
Thank you very much :D |
|
Awesome thanks guys :) |
miglesiasEB
pushed a commit
that referenced
this issue
May 31, 2019
<!--- Provide a general summary of your changes in the Title above --> ## Description <!--- Describe your changes in detail --> The resizeMainHorizontal was added in the PR #84 as a hacking fix for the responsive container in the project documentation. This function broke the charts responsiveness outside the docs because it was looking for a `main` tag present in the **React Styleguidist** docs but not in every other project. This PR removes the problematic function and uses the correct one to fix the problem. The doc's responsiveness was tested using the original function and it work ok with this version of React Styleguidist. This PR also address the issue #144 to make it easier to import the ResponsiveContainer component and the withResponsiveness helper. ## Motivation and Context <!--- Why is this change required? What problem does it solve? --> <!--- If it fixes an open issue, please link to the issue here. --> * The charts' responsiveness is currently broken as described in the issue #105. * At the moment is difficult to import the ResponsiveContainer component and the withResponsiveness helper because you have to do it directly from the ESM bundle. ## How Has This Been Tested? <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> * All the unit tests were ran and all of them passed. * The documentation pages were reviewed to check for any inconsistency with the published one. ## Screenshots (if appropriate): <img width="1676" alt="wide" src="https://user-images.githubusercontent.com/282903/49551467-5007dc80-f8bd-11e8-9611-53de3d8166c9.png"> <img width="1237" alt="narrow" src="https://user-images.githubusercontent.com/282903/49551475-58601780-f8bd-11e8-9fd6-eaf84ecaf96e.png"> ## Types of changes <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] Refactor (changes the way we code something without changing its functionality) - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Checklist: <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have read the **CONTRIBUTING** document. - [ ] I have updated the documentation accordingly. - [ ] I have added tests to cover my changes. - [x] All new and existing tests passed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hi Guys. Could you advise on the intended way to import the
withResponsivenesshelper?Current Behaviour
In the docs it uses a relative import, which I assume is because you compile your docs examples (nice)
I've found the compiled version in
britecharts-react/lib/esm/helpersand the below code works fine, but is quite long-winded:Expected Behaviour
I would like to import
withResponsiveness()from the root, so:Possible Solution
Happy to submit a PR to fix this, just let me know :)
Your Environment
0.4.1with TypeScript 3.1 and React 16.6The text was updated successfully, but these errors were encountered: