-
-
Notifications
You must be signed in to change notification settings - Fork 532
Completion of task #1285
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
Completion of task #1285
Conversation
i have successfully completed the task, please view and confirm it
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit c8f7921:
|
| import React from "react"; | ||
| import React, { useState } from "react"; | ||
| import { IconButton, ActionButton, CommandBarButton, DefaultButton } from "@fluentui/react"; | ||
| import JSONEditorModal from './JSONEditorModal'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should not change fluent package for this task, only demo app should be affected
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1285 +/- ##
=======================================
Coverage 79.62% 79.63%
=======================================
Files 246 246
Lines 13145 13143 -2
Branches 1743 1743
=======================================
- Hits 10467 10466 -1
+ Misses 1800 1799 -1
Partials 878 878 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| const fields: Fields = { | ||
| name: { | ||
| label: "Name", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes to the config are not needed for this task
| "brace-expansion@2.0.1": "^2.0.2" | ||
| } | ||
| }, | ||
| "onlyBuiltDependencies": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the purpose of these changes?
| const importFromJsonLogic = (jsonLogicStr: string) => { | ||
| try { | ||
| const jsonLogic = JSON.parse(jsonLogicStr); | ||
| const tree = Utils.loadFromJsonLogic(jsonLogic, state.config); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use
const [tree, jsonLogicErrors] = Utils._loadFromJsonLogic(jsonLogic as JsonLogicTree, state.config);
And put jsonLogicErrors into state
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address comments above
|
no problem , i will address them now |
|
i just push now, please confirm that it is the task you want completed |
|
Thanks @johnkennedyb
Screen.Recording.2025-07-25.at.13.04.10.movAlso please run |
|
okay , is that all you need for the task completion ?, every other thing is up to your taste , if that is all, i will work on it |
|
the task has been completed |
|
it has been completed @ukrbublik |
i have successfully completed the task, please view and confirm it