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

Added Feature Cover Json Data Node in Data Node control #2164

Conversation

THEBOSS0369
Copy link
Contributor

@THEBOSS0369 THEBOSS0369 commented Oct 26, 2024

This PR fixes #1248

Hey Everyone!

In this PR, The changes has been done in DataNodeViewer.tsx file, Now Data in Json can be collapsed or expand as the user wish.
I have imported react-json-view because it will make our process much easier.

Thanks!

@THEBOSS0369
Copy link
Contributor Author

@quest-bot loot #1248

Copy link

quest-bot bot commented Oct 26, 2024

Quest PR submitted! image Quest PR submitted!

@THEBOSS0369, you are attempting to solve the issue and loot this Quest. Will you be successful?


Questions? Check out the docs.

@quest-bot quest-bot bot added the ⚔️ Quest Tracks quest-bot quests label Oct 26, 2024
@quest-bot quest-bot bot mentioned this pull request Oct 26, 2024
4 tasks
Copy link
Member

@FredLL-Avaiga FredLL-Avaiga left a comment

Choose a reason for hiding this comment

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

Needs tests

frontend/taipy/src/DataNodeViewer.tsx Outdated Show resolved Hide resolved
frontend/taipy/src/DataNodeViewer.tsx Outdated Show resolved Hide resolved
frontend/taipy/src/DataNodeViewer.tsx Outdated Show resolved Hide resolved
@THEBOSS0369 THEBOSS0369 force-pushed the feature/#1248-cover-json-data-node-control-new branch from 12fb4e6 to 7daba5e Compare October 29, 2024 15:22
@THEBOSS0369
Copy link
Contributor Author

Needs tests

Hey @FredLL-Avaiga ! Apologies for late reply. I have created a test file can you tell me in which folder should i add that file , this is the test file in the root folder

image

@FredLL-Avaiga
Copy link
Member

Needs tests

Hey @FredLL-Avaiga ! Apologies for late reply. I have created a test file can you tell me in which folder should i add that file , this is the test file in the root folder

image

What does this show ? that you can display some json data in a markdown page ?
I don't understand how this would use the data_node control ?

Copy link
Member

@FredLL-Avaiga FredLL-Avaiga left a comment

Choose a reason for hiding this comment

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

this needs:

  • backend check and modification if necessary
  • tests
  • example of the data_node control usage with json

@FredLL-Avaiga
Copy link
Member

please fix the failing checks

@THEBOSS0369
Copy link
Contributor Author

please fix the failing checks

Yes sir i will fix these errors.

@THEBOSS0369 THEBOSS0369 force-pushed the feature/#1248-cover-json-data-node-control-new branch from 58feaaa to 00272c1 Compare October 31, 2024 07:13
@THEBOSS0369
Copy link
Contributor Author

THEBOSS0369 commented Oct 31, 2024

@FredLL-Avaiga The test is failing because of the <Grid > component as in file it has been imported from @mui/material/Grid2
and when i'm using it this error is showing
image

it can resolve if i import it from @mui/material/Grid which is definitely not good option , so can you tell me how to use Grid in this file.

@THEBOSS0369
Copy link
Contributor Author

  • example of the data_node control usage with json

Sir, I will be adding this in the next commit

@FredLL-Avaiga
Copy link
Member

@FredLL-Avaiga The test is failing because of the <Grid > component as in file it has been imported from @mui/material/Grid2
and when i'm using it this error is showing
image

it can resolve if i import it from @mui/material/Grid which is definitely not good option , so can you tell me how to use Grid in this file.

You have to update your dependencies
Grid2 comes with Mui6 which is what we use in Taipy

@THEBOSS0369
Copy link
Contributor Author

@FredLL-Avaiga The test is failing because of the <Grid > component as in file it has been imported from @mui/material/Grid2
and when i'm using it this error is showing
image

it can resolve if i import it from @mui/material/Grid which is definitely not good option , so can you tell me how to use Grid in this file.

You have to update your dependencies
Grid2 comes with Mui6 which is what we use in Taipy

Got it Sir , This will work! Also I won't be able to share any update till tomorrow due to Diwali Festival. Hope you understand 😀

@FredLL-Avaiga
Copy link
Member

Also I won't be able to share any update till tomorrow due to Diwali Festival. Hope you understand 😀

Enjoy the festival 🙏

Copy link
Member

@FredLL-Avaiga FredLL-Avaiga left a comment

Choose a reason for hiding this comment

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

to make things work, you will nee to modify the backend also:
in _adapters.py

return isinstance(datanode, _TabularDataNodeMixin) or isinstance(

the static method should be

    @staticmethod
    def _is_tabular_data(datanode: DataNode, value: t.Any):
        return isinstance(datanode, _TabularDataNodeMixin) or (
            isinstance(value, (pd.DataFrame, pd.Series, list, tuple, dict)) and not isinstance(datanode, JSONDataNode)
        )

PS do not forget to import JSONDataNode
from taipy.core.data import JSONDataNode

doc/gui/examples/controls/collapse_data_view.py Outdated Show resolved Hide resolved
Copy link
Member

@FredLL-Avaiga FredLL-Avaiga left a comment

Choose a reason for hiding this comment

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

You should also have a change in package.json ?
and the package react-json-view is not maintained any more ... need to change it

frontend/taipy/src/DataNodeViewer.tsx Outdated Show resolved Hide resolved
@THEBOSS0369 THEBOSS0369 force-pushed the feature/#1248-cover-json-data-node-control-new branch from 2c58cec to 6e8b61d Compare November 1, 2024 05:48
@THEBOSS0369
Copy link
Contributor Author

THEBOSS0369 commented Nov 1, 2024

@FredLL-Avaiga ! As react-json-view is not maintained now so i have imported react-json-tree which is maintained regularly
and takes very much less space comparted to react-json-view i have created 2 files to import that because when i was lazy importing it i was getting many errors , to avoid that i divided in file LazyReactJsonTree.tsx and react-json-tree.ds. All the errors that i was getting get resolved by this

I created a demo-1248-dn.json file for the demo i have added the same file in root folder for testing as well, but don't know why its not showing when i try to add it in the commit here is the ss for it, I think i am adding that file in wrong folder maybe that is causing problem?
image

the output is coming something like this
image

@THEBOSS0369 THEBOSS0369 force-pushed the feature/#1248-cover-json-data-node-control-new branch from 4ec5603 to 130e3ab Compare November 1, 2024 06:06
@FredLL-Avaiga
Copy link
Member

We don't want any demo-* fille in the repo
But at least you can run it and test the result of your changes sand debug the frontend if necessary.

@FredLL-Avaiga
Copy link
Member

By the screen shot it doesn't look like the json is rendered?

@THEBOSS0369
Copy link
Contributor Author

THEBOSS0369 commented Nov 1, 2024

By the screen shot it doesn't look like the json is rendered?

Yes sir I don't know what am i doing wrong there is a popup comes when i hover over Data that shows blocked by me . i don't understand why its coming
image

i also tried it by changing shouldExpandNode={() => false} to true still nothing changes. Can you help me with what am i doing wrong

@FredLL-Avaiga
Copy link
Member

FredLL-Avaiga commented Nov 7, 2024

I think you will see that :

  • JSonViewer does not take a collapse property
  • it does that a theme property that you can set to theme.palette.mode after getting the current theme by const theme = useTheme();
  • you need to block the edition mode for now with
                              <Grid
                                        container
                                        size={12}
                                        justifyContent="space-between"
                                        data-focus={dataValueFocus}
                                        onClick={dtType == "dict" ? undefined: onFocus}
                                        sx={hoverSx}
                                    >

around

ie replacing the onClick handler with undefined when dtType == "dict"

@THEBOSS0369 THEBOSS0369 force-pushed the feature/#1248-cover-json-data-node-control-new branch from 0eb3324 to 3187dc9 Compare November 8, 2024 09:59
@THEBOSS0369
Copy link
Contributor Author

Hey @FredLL-Avaiga ! Can you take a look now.

@FredLL-Avaiga
Copy link
Member

Hey @FredLL-Avaiga ! Can you take a look now.

Can you fix the checks ?
Property 'collapsed' does not exist on type 'IntrinsicAttributes & JsonViewerProps<unknown>'.

@THEBOSS0369
Copy link
Contributor Author

@FredLL-Avaiga ! I think this problem is occuring because maybe there is no collapsed function showing in the main lib . So shall i go ahead and use any other lib like this one https://www.npmjs.com/package/react-json-view-lite
image

@FredLL-Avaiga
Copy link
Member

No just remove the collapse properly

@THEBOSS0369
Copy link
Contributor Author

@FredLL-Avaiga ! It seems all tests have passed what to do next?

FredLL-Avaiga
FredLL-Avaiga previously approved these changes Nov 8, 2024
Copy link
Member

@FredLL-Avaiga FredLL-Avaiga left a comment

Choose a reason for hiding this comment

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

Let's see if @FabienLelaquais liked it

@FredLL-Avaiga FredLL-Avaiga added Core Related to Taipy Core 🖰 GUI Related to GUI 🟨 Priority: Medium Not blocking but should be addressed ✨New feature 📝Release Notes Impacts the Release Notes or the Documentation in general ✅ hacktoberfest: approved An approved PR for Hacktoberfest rewarding labels Nov 8, 2024
@FredLL-Avaiga
Copy link
Member

Can you merge this @jrobinAV ?

@THEBOSS0369
Copy link
Contributor Author

Hey @FredLL-Avaiga ! There was an extra comma in json file which was not letting the test passed now i have removed it and test are Passing.


json_config_node = Config.configure_json_data_node(
id="json_node",
default_path="./demo-1248-dn.json",
Copy link
Member

Choose a reason for hiding this comment

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

where is the json file ?

Copy link
Member

Choose a reason for hiding this comment

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

I guess it should be renamed datanode_viewer_json.json
And that would be why the demo-* file was ignored

@THEBOSS0369 THEBOSS0369 force-pushed the feature/#1248-cover-json-data-node-control-new branch from 6e47af1 to c5b6fa9 Compare November 9, 2024 14:12
@THEBOSS0369
Copy link
Contributor Author

@FredLL-Avaiga Sir all tests passed if its ready to merge please merge this. Otherwise merge conflict will create the issue 🙂

Copy link
Member

@FredLL-Avaiga FredLL-Avaiga left a comment

Choose a reason for hiding this comment

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

Another try @jrobinAV

@jrobinAV jrobinAV merged commit 1486c44 into Avaiga:develop Nov 13, 2024
53 of 56 checks passed
@jrobinAV jrobinAV added hacktoberfest hacktoberfest issues hacktoberfest - 300💎💎💎 Issues rewarded by 300 points labels Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Related to Taipy Core 🖰 GUI Related to GUI hacktoberfest - 300💎💎💎 Issues rewarded by 300 points ✅ hacktoberfest: approved An approved PR for Hacktoberfest rewarding hacktoberfest hacktoberfest issues ✨New feature 🟨 Priority: Medium Not blocking but should be addressed ⚔️ Quest Tracks quest-bot quests 📝Release Notes Impacts the Release Notes or the Documentation in general
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cover JSON data in Data node control
3 participants