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

Issue #338: Added onEvaluatedDataChange prop to expose the result of the formula #379

Merged
merged 6 commits into from
Mar 6, 2024

Conversation

BALURAM1226
Copy link
Contributor

Issue #338: Added getEvaluatedData prop to expose the result of the formula

src/Spreadsheet.tsx Outdated Show resolved Hide resolved
BALURAM1226 and others added 2 commits February 14, 2024 20:11
Co-authored-by: Iddan Aaronsohn <mail@aniddan.com>
Rename prop function name getEvaluatedData to onEvaluatedDataChange
@iddan
Copy link
Owner

iddan commented Feb 14, 2024

Please see CI failure. You can also run it locally with yarn ci

Adjust space of new changed code
@BALURAM1226
Copy link
Contributor Author

I tried Prettier ESLint It worked. let's try to merge again

@BALURAM1226
Copy link
Contributor Author

@iddan I have committed code with Prettier that resolved a CI failure issue. Can you please review the code?

@BALURAM1226
Copy link
Contributor Author

@iddan , I have sent a review request for changes to address the resolved CI failure. Please take a moment to review.Thank you

@iddan
Copy link
Owner

iddan commented Feb 22, 2024

Thank you, seems like CI is still failing

@BALURAM1226
Copy link
Contributor Author

Screenshot 2024-02-22 210043
Screenshot 2024-02-22 211147

@iddan, I ran npm run check-format in my local code and identified code style issues in 96 files. After running npm run format, I resolved the code style issues in all 96 files. However, I only committed the changes to the Spreadsheet.tsx file , because in CI failure only in Spreadsheet.tsx file code style issue was found. I believe this should resolve the issue. If you have any other suggestions, please let me know

@BALURAM1226 BALURAM1226 changed the title Issue #338: Added getEvaluatedData prop to expose the result of the formula Issue #338: Added onEvaluatedDataChange prop to expose the result of the formula Feb 22, 2024
@BALURAM1226
Copy link
Contributor Author

Screenshot 2024-02-29 201049

@iddan After running the 'npm run ci' command locally, I encountered an error. After fixing the above CI failure ( Property 'onEvaluatedDataChange' is missing in type ), should I push the commit that resolved the CI failure? Can you please provide suggestions regarding this below error? May this below error affect to CI ?

Error: No files matching the pattern "'*/**.css'" were found.
at C:\New folder\react-spreadsheet\node_modules\stylelint\lib\standalone.js:212:12
ERROR: "lint-css" exited with 1.
ERROR: "lint" exited with 1.

@iddan
Copy link
Owner

iddan commented Mar 2, 2024

I see TypeScript errors:
Screen Shot 2024-03-02 at 18 34 33

@BALURAM1226
Copy link
Contributor Author

@iddan Yes, you are correct. I resolved the code issue in my local system. After that, I ran 'npm run CI' locally, and I encountered an error, as you can see in the screenshot above. I am seeking confirmation from you, and if the above error does not affect the CI failure, then I will commit the new code. I've been facing CI failures repeatedly, so if the mentioned error doesn't impact the CI failure, I'll proceed to commit the new code and try again for a successful merge

@iddan
Copy link
Owner

iddan commented Mar 2, 2024

You can push, and if the CI command passes in GitHub Actions it's ok

@coveralls
Copy link

Pull Request Test Coverage Report for Build 8166588409

Details

  • 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.03%) to 82.51%

Totals Coverage Status
Change from base Build 7684993603: -0.03%
Covered Lines: 1143
Relevant Lines: 1317

💛 - Coveralls

@iddan iddan merged commit b061c81 into iddan:master Mar 6, 2024
3 of 4 checks passed
@BALURAM1226
Copy link
Contributor Author

@iddan Do you need to release a new version to make these changes available to all npm package users? Because added new props visible in storybook and that working but not show in npm package after installation

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

3 participants