Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Activity
    Joshua Denzau
    @joshdenz
    Hey guys. Just an FYI here, your dependencies has 1 high severity vulnerability requiring an update to webpack-dev-server v3.1.14
    Joshua Denzau
    @joshdenz
    I'm thinking we may need to rearrange the project structure when it comes to the tests. Mocha does not look for tests in a tests directory by default, and you have told mocha to look for tests in 'src/components/**/*.test.js'. So either we need to do something like tell Mocha to look for tests in 'src/components/**/__tests__/*.test.js' or change where the tests are located in the project structure.
    Joshua Denzau
    @joshdenz
    also one last thing regarding test location, in the package.json file you need to use double quotes for the test path rather than single quotes. You've got to use the escape char for double quotes so you can use two sets of them on your test line.
    Rishi Kumar Chawda
    @rishichawda
    @joshdenz It does look for test files in __test__ directory. Since src/components/**/*.test.jsserves the same purpose as src/components/**/__tests__/*.test.js in our case as we are going to put test files only in the __tests__ folder. However, if we would like to keep it self explanatory, then we can do something like src/components/*/__tests__/*.test.js but I don't really feel like it is necessary since we're going to maintain the structure anyways.

    also one last thing regarding test location, in the package.json file you need to use double quotes for the test path rather than single quotes. You've got to use the escape char for double quotes so you can use two sets of them on your test line.

    @joshdenz using single / double quotes really don't matter since both denote strings in shell.

    Hey guys. Just an FYI here, your dependencies has 1 high severity vulnerability requiring an update to webpack-dev-server v3.1.14

    A low severity issue that has been popping up every now and then in repositories. I'll update them though. Thanks for reminding!

    Rishi Kumar Chawda
    @rishichawda
    @joshdenz incase you are still not sure wether mocha is looking for test files in __tests__ directory, you can simply run npm run test which will show you one pending test from button render test - which will confirm wether mocha is able to find the test files.
    Also, I was thinking about having separate test files for each category, as in , render.test.js for render tests, accessibility.test.js for accessibility tests, edgecase.test.js for edge case tests ( if any ) and so on.
    @joshdenz @KRRISH96 what are your thoughts on that?
    Joshua Denzau
    @joshdenz
    The things I suggested were the things that were required to actually run the tests in my environment. After cloning the repo and installing dependencies running npm test resulted in no tests being found. I needed to explicitly specify the __tests__ folder in package.json and I needed to use escaped double quotes for the path else no tests were found.
    My apologies if everything was running smoothly in your environment.
    Rishi Kumar Chawda
    @rishichawda
    Mm.. seems weird. you can change it then. which env are you on, by the way?
    Rishi Kumar Chawda
    @rishichawda
    And also, checkout from development branch.
    Joshua Denzau
    @joshdenz
    I'm on windows 10
    Rishi Kumar Chawda
    @rishichawda
    okay.
    Did you start writing tests?
    I'm working on more components.
    Joshua Denzau
    @joshdenz
    I have started working on tests. I have started with the button component. But now, it is time for bed. Have a nice day!
    Joshua Denzau
    @joshdenz
    oh @rishichawda regarding the render.test.js accessibility.test.js edgecase.test.js convention, I don't see a problem with that. That seems like an appropriate split to make.
    Rishi Kumar Chawda
    @rishichawda
    Cool. we'll go with that then.
    @KRRISH96 has started working on the library too and we're also looking for more developers online to start contributing. Hoping to see the activity increase in the repository.
    Joshua Denzau
    @joshdenz
    It's odd that the test structure didn't work for me as you had initially written it, when it worked for you.
    Rishi Kumar Chawda
    @rishichawda
    Yeah. Works in macOS and linux. I haven't used windows before.
    Joshua Denzau
    @joshdenz
    I'm fairly certain that the double quotes issue in package.json is caused by Windows being Windows lol
    Joshua Denzau
    @joshdenz
    Actually I think this github issue describes the problem I am seeing mochajs/mocha#2138
    Rishi Kumar Chawda
    @rishichawda
    Ah. This explains why the original command wasn't able to find the test files.
    Rishi Kumar Chawda
    @rishichawda
    @joshdenz How's it going?
    Joshua Denzau
    @joshdenz
    @rishichawda to be honest I haven't looked at any code in a couple of days as my wife and daughter have the flu and I have been taking care of them.
    Rishi Kumar Chawda
    @rishichawda
    @joshdenz Oh, take care guys.
    Joshua Denzau
    @joshdenz
    Thanks. It has been a trying time. Everyone is getting better though slowly. Thanks for the kind words.
    Joshua Denzau
    @joshdenz
    I've had a chance to look at this again and I'm running into an issue with the Button component. If you render the button with type="light" the color should be rendered as color.dark but instead you get the defaultProp color.light
    Rishi Kumar Chawda
    @rishichawda
    the color in the button's context actually refers to the text color. As it is the case with html element - color refers to the text color. For button's color, there is a separate prop named bg.
    Do the prop names look confusing? @joshdenz @KRRISH96
    Joshua Denzau
    @joshdenz
    no no, I know that... what I mean is that if you render the button with type="light" it should render the color as color.dark... meaning the text color should be dark to contrast the light bg color
    but instead the text color is rendered as color.light
    so the button ends up white with white text
    Rishi Kumar Chawda
    @rishichawda
    Oh. Let me see.
    Rishi Kumar Chawda
    @rishichawda
    @joshdenz I've fixed that. will raise a pr and merge it on development.
    You can pull latest development branch and work on it.
    Joshua Denzau
    @joshdenz
    what was the issue, Im curious?
    Rishi Kumar Chawda
    @rishichawda
    was a typo in the code. had it wrapped in a parenthesis 😅
    Joshua Denzau
    @joshdenz
    haha, of course it was a typo facepalm
    Rishi Kumar Chawda
    @rishichawda
    You can pull development now. I've raised the PR and merged. this is the commit with fix : 550f4d4
    sorry about being so late. was on a call
    Joshua Denzau
    @joshdenz
    its ok, I'm home today with no deadlines so I'm very relaxed :)
    Rishi Kumar Chawda
    @rishichawda
    great! how's your SO and baby? are they fine now?
    Joshua Denzau
    @joshdenz
    yes, everyone is healthy again. Out of 30 kids in my daughters class, last week 20 of them including 2/3rds of the teachers were sick
    pneumonia, flu, fever... its been a mess
    but thankfully my girls are doing well
    Rishi Kumar Chawda
    @rishichawda
    Good to hear. God bless you guys. Take care.
    Joshua Denzau
    @joshdenz
    thanks again for the kind words