Skip to content

Conversation

@johnkennedyb
Copy link

i have successfully completed the task, please view and confirm it

i have successfully completed the task, please view and confirm it
@codesandbox
Copy link

codesandbox bot commented Jul 24, 2025

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@vercel
Copy link

vercel bot commented Jul 24, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-awesome-query-builder-examples ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2025 5:19pm
react-awesome-query-builder-sandbox ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2025 5:19pm
react-awesome-query-builder-sandbox-next ✅ Ready (Inspect) Visit Preview Jul 25, 2025 5:19pm

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 24, 2025

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:

Sandbox Source
@react-awesome-query-builder/examples Configuration
@react-awesome-query-builder/sandbox Configuration
@react-awesome-query-builder/sandbox-simple Configuration
@react-awesome-query-builder/sandbox-next Configuration

import React from "react";
import React, { useState } from "react";
import { IconButton, ActionButton, CommandBarButton, DefaultButton } from "@fluentui/react";
import JSONEditorModal from './JSONEditorModal';
Copy link
Owner

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
Copy link

codecov bot commented Jul 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.63%. Comparing base (2fc26f3) to head (7a95fc2).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.


const fields: Fields = {
name: {
label: "Name",
Copy link
Owner

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": [
Copy link
Owner

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);
Copy link
Owner

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

Copy link
Owner

@ukrbublik ukrbublik left a 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

@johnkennedyb
Copy link
Author

no problem , i will address them now

this is a fix to the previous issues
@johnkennedyb
Copy link
Author

i just push now, please confirm that it is the task you want completed

@ukrbublik
Copy link
Owner

ukrbublik commented Jul 25, 2025

Thanks @johnkennedyb
I found 2 issues with UX:

  1. If I want to resize textarea in modal (to make it bigger horizontally and vertically) I won't be able to do this. Could you fix this, or make textarea bigger (to occupy 80% of height, and remove max-width: 600px;)

  2. See video. If I hold mouse to select text and release mouse outside of modal, modal would close

Screen.Recording.2025-07-25.at.13.04.10.mov

Also please run pnpm lint-fix

@johnkennedyb
Copy link
Author

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

@johnkennedyb
Copy link
Author

the task has been completed
@ukrbublik

@johnkennedyb
Copy link
Author

johnkennedyb commented Jul 25, 2025

it has been completed @ukrbublik

@ukrbublik ukrbublik merged commit c8f7921 into ukrbublik:master Jul 25, 2025
6 checks passed
ukrbublik pushed a commit that referenced this pull request Jul 25, 2025
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.

2 participants