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

feat: Add info panel item panel to load and display data on demand #2622

Merged
merged 13 commits into from
Jan 30, 2025

Conversation

vardhan0604
Copy link
Contributor

New Pull Request Checklist

Issue Description

Closes: #2610

Approach

TODOs before merging

Copy link

parse-github-assistant bot commented Oct 23, 2024

Thanks for opening this pull request!

  • 🎉 We are excited about your hands-on contribution!

@vardhan0604
Copy link
Contributor Author

Screen.Recording.2024-10-24.033012.mp4

Hey @mtrezza, I have added a subset element that allows loading another Cloud Code function on demand. I’m not sure about the styles; could you please let me know what changes and features are required after reviewing the current progress?

Copy link

uffizzi-cloud bot commented Oct 23, 2024

Uffizzi Ephemeral Environment deployment-57571

⌚ Updated Oct 23, 2024, 22:03 UTC

☁️ https://app.uffizzi.com/github.com/parse-community/parse-dashboard/pull/2622

📄 View Application Logs etc.

What is Uffizzi? Learn more

@mtrezza
Copy link
Member

mtrezza commented Oct 23, 2024

That is amazing! Just what I had in mind. From a first look I things all that's needed is to refine the design of the section. Let me come up with a mock up... Nice work! Maybe @404-html wants to give this a quick review as well.

@404-html
Copy link
Member

Hi again Gents! 👋

Maybe @404-html wants to give this a quick review as well.

With pleasure!

@@ -95,3 +97,116 @@ export const ButtonElement = ({ item, showNote }) => {
</div>
);
};

export const PanelElement = ({ item, showNote, objectId, depth = 0 }) => {
Copy link
Member

Choose a reason for hiding this comment

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

Instead of introducing another PanelElement I would suggest to extend existing one (in AggregationPanel.js), and adopt it to loading data subset and being used recessively. This way we should avoid code duplication.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have tried doing in the latest commits, can you please review it?

@vardhan0604
Copy link
Contributor Author

Hi @404-html, I have updated the code as requested. Could you please review it?

@404-html
Copy link
Member

404-html commented Oct 25, 2024

Hi @404-html, I have updated the code as requested. Could you please review it?

Let me wait for Manuel's UI suggestions, and after they will be addressed I will do another review.

@mtrezza
Copy link
Member

mtrezza commented Oct 25, 2024

Not sure why, but for some reason I cannot get the "Show panel" button to show, despite running the same setup like in the previous PR where it worked. @vardhan0604 could you try to rebuild it with the latest commit here and see whether it still works for you?

@vardhan0604
Copy link
Contributor Author

vardhan0604 commented Oct 26, 2024

Not sure why, but for some reason I cannot get the "Show panel" button to show, despite running the same setup like in the previous PR where it worked. @vardhan0604 could you try to rebuild it with the latest commit here and see whether it still works for you?

Yeah, it is working for me. I rebuilt it with the latest commit, and the 'Show panel' button appears as expected. @mtrezza, could you check if parse-dashboard-config.json is configured properly?

@mtrezza
Copy link
Member

mtrezza commented Oct 29, 2024

Still trying to get this to work. Meanwhile I found this:

const Stats = ({ data, classwiseCloudFunctions, className }) => {

Only remotely related to this PR, but maybe we can clean it up at once. Why was classwiseCloudFunctions added to Stats? If I understand it correctly, classwiseCloudFunctions is just for the info panel, but Stats is a completely different feature?

@mtrezza
Copy link
Member

mtrezza commented Oct 29, 2024

#2623 is the reason why the "Show Panel" button isn't showing. Could you take a look?

@@ -591,7 +591,7 @@ export default class DataBrowser extends React.Component {
<ResizableBox
width={this.state.panelWidth}
height={Infinity}
minConstraints={[100, Infinity]}
minConstraints={[400, Infinity]}
Copy link
Member

Choose a reason for hiding this comment

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

Please revert if not required for this PR; the panel should be resizable to min. 100 px width.

panelWidth: 300,
panelWidth: 400,
Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, both of them were unintentional changes, i will revert them back in next commit.

@mtrezza
Copy link
Member

mtrezza commented Oct 30, 2024

Here's a suggestion for a slight UI refinement:

Collapsed:

image

Expanded:

image

Notes

  • I've tried to better distinguish the expandable section from a segment header by using a different background color, but defining colors may be a future feature, so maybe it's better to avoid assuming certain color combinations but rather focus on form. Anyway, here's how it would look like in a less-intrusive gray for example:

    image
  • When expanded, the sub-panel is encompassed by a 1px gray line to the left and bottom, to visualize the information hierarchy in the panel.

  • As we can see in the expanded state, there are 2 "Purchases" headlines right below each other, 1 from the expandable sub-panel item, 1 from the title of the segment within the sub-panel. In cases where this is not desired, the title "Purchases" of the first segment can be avoided by simply not specifying a segment title. That feature doesn't exist yet as the title field is a mandatory property of a segment, but it will be easy to add in a future PR.

  • I have used the same design language for expandable sections as we already do in the data browser classes for saved filters:

    • Collapsed:
      image

    • Expanded:
      image

@vardhan0604
Copy link
Contributor Author

#2623 is the reason why the "Show Panel" button isn't showing. Could you take a look?

Yeah sure. I will take a look

@vardhan0604
Copy link
Contributor Author

vardhan0604 commented Nov 2, 2024

#2623 is the reason why the "Show Panel" button isn't showing. Could you take a look?

@mtrezza, should I push the changes here or open a new PR? I’ve already fixed it locally and also added the UI changes.

@mtrezza
Copy link
Member

mtrezza commented Nov 3, 2024

New PR please. It's a fix unrelated to this new feature.

@vardhan0604
Copy link
Contributor Author

vardhan0604 commented Nov 3, 2024

New PR please. It's a fix unrelated to this new feature.

@mtrezza can you review it, I have opened the PR #2627

Also, I’ve implemented the changes according to the UI provided and pushed the changes in this PR. Could you please review and let me know if any additional adjustments are needed.

@vardhan0604
Copy link
Contributor Author

Also, I’ve implemented the changes according to the UI provided and pushed the changes in this PR. Could you please review and let me know if any additional adjustments are needed.

Hey @mtrezza ,
Since PR #2627 has already been resolved, I’d appreciate it if you could review this one.
Thank you!

@mtrezza
Copy link
Member

mtrezza commented Jan 28, 2025

Sure, I'll take a look at #2622 first in a bit.

@mtrezza
Copy link
Member

mtrezza commented Jan 29, 2025

Well done!

The only minor issues I've discovered:

a) The horizontal indentation increases with every sub panel, but should stay the same:

image

b) The reload icon is gray instead of white:
image

@vardhan0604
Copy link
Contributor Author

Hey @mtrezza, I have fixed the requested changes.
This is how it looks now.
image

Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

Looks good! Nice work

@mtrezza mtrezza changed the title feat: Add element for subset of data to info panel feat: Add info panel item panel to load and display data on demand Jan 30, 2025
@mtrezza mtrezza merged commit 8e5741d into parse-community:alpha Jan 30, 2025
10 checks passed
parseplatformorg pushed a commit that referenced this pull request Jan 30, 2025
# [6.0.0-alpha.25](6.0.0-alpha.24...6.0.0-alpha.25) (2025-01-30)

### Features

* Add info panel item `panel` to load and display data on demand ([#2622](#2622)) ([8e5741d](8e5741d))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.0.0-alpha.25

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Jan 30, 2025
@mtrezza
Copy link
Member

mtrezza commented Jan 30, 2025

@vardhan0604 We forgot to add the new item type to the README info panel section. Would you mind opening a quick PR for that?

@mtrezza
Copy link
Member

mtrezza commented Jan 31, 2025

I've added the docs with #2646, no TODO left.

@vardhan0604
Copy link
Contributor Author

I've added the docs with #2646, no TODO left.

Great, thanks for handling that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released-alpha Released as alpha version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add element for subset of data to info panel
4 participants