Skip to content

Conversation

@Tejas-ChandraShekarRaju
Copy link
Contributor

@Tejas-ChandraShekarRaju Tejas-ChandraShekarRaju commented Oct 17, 2022

  1. Ran prettier on files.
  2. Enhanced sort feature to be enabled when a user clicks on column headers than clicking on the arrows. Suggested by @DeRaowl

Fixes (#69)

Enhancement

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Manual Testing
  • Unit test cases - Pending

Test Configuration:

  • Firmware version:
  • Hardware:
  • Toolchain:
  • SDK:

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@Tejas-ChandraShekarRaju Tejas-ChandraShekarRaju added the enhancement Improving something existing label Oct 17, 2022
@Tejas-ChandraShekarRaju Tejas-ChandraShekarRaju linked an issue Oct 17, 2022 that may be closed by this pull request
Copy link
Contributor

@prakashchoudhary07 prakashchoudhary07 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

/>
)}

No newline at end of file
return <Table featureFlags={featureFlags} headers={headers} />;
Copy link

Choose a reason for hiding this comment

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

can we just make use of arrow function here? No need to write return explicitly

Copy link
Contributor Author

@Tejas-ChandraShekarRaju Tejas-ChandraShekarRaju Nov 4, 2022

Choose a reason for hiding this comment

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

Same is followed everywhere. This is not a change from this PR. It shows it changed because i ran perttier. Let me know if you still need me to change it because all components follow the same

Copy link

Choose a reason for hiding this comment

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

Ok, we can change this to arrow function

? a[key].toUpperCase() : a[key];
const varB = (typeof b[key] === 'string')
? b[key].toUpperCase() : b[key];
const varA = typeof a[key] === "string" ? a[key].toUpperCase() : a[key];
Copy link

Choose a reason for hiding this comment

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

Can we have better naming for variable varA and varB doesn't describe it's purpose

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @DeRaowl this is a comparator function. What do you think would be better? Since those are keys can i name them key1 and key2?

Copy link

Choose a reason for hiding this comment

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

Ok instead you can just add small comment saying what these variables are

/>
)}

No newline at end of file
return <Table featureFlags={featureFlags} headers={headers} />;
Copy link

Choose a reason for hiding this comment

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

Ok, we can change this to arrow function

? a[key].toUpperCase() : a[key];
const varB = (typeof b[key] === 'string')
? b[key].toUpperCase() : b[key];
const varA = typeof a[key] === "string" ? a[key].toUpperCase() : a[key];
Copy link

Choose a reason for hiding this comment

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

Ok instead you can just add small comment saying what these variables are

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improving something existing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clean the blank lines for sort

3 participants