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

feature: [dev-8574] Advanced Editor updates #1372

Merged
merged 3 commits into from
Jul 11, 2024

Conversation

joshlacey
Copy link
Collaborator

We needed a way to more easily configure and edit configs directly in COVE. I've added a upload config button to the first tab of the editor
custom upload

I've also added a new component to the Advanced Editor https://carlosnz.github.io/json-edit-react/ which allows collapse and expand to more granularly edit configs.
advanced-editor

I've also enhanced the editor to prefilter the config so you're only seeing what will eventually be saved. Before there were temporary data states present which made the config difficult to scroll through.

Copy link
Collaborator

@adamdoe adamdoe left a comment

Choose a reason for hiding this comment

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

I think the only issue that needs to be resolved here is the apply button. I'm not able to change the config using the new advanced editor at the moment. I think we'll also want to weight the pros/cons of the speed of the advanced editor vs the text based one.

packages/core/components/AdvancedEditor/AdvancedEditor.tsx Outdated Show resolved Hide resolved
@@ -309,6 +326,21 @@ export default function ChooseTab() {
</Tooltip>
</li>
</ul>
<hr />
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks good and is working for me. When I used this option I did notice that the Advanced Settings seemed to run really slow. I'm not sure if it was the config I tried uploading. However, compared to the text based format this does seem slower. I think we'll want to weigh pros/cons with speed with text based editor and using this library.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think maybe because it has to also do a full data pull with this option? with the text editor you're changing things after the data has been updated, which i also thought was no ideal UX because you can also change the data there.

@joshlacey joshlacey requested a review from adamdoe June 25, 2024 15:06
Copy link
Collaborator

@adamdoe adamdoe left a comment

Choose a reason for hiding this comment

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

When I paste in an example dashboard config the visualizations property is being stripped and returns an empty dashboard.

@joshlacey
Copy link
Collaborator Author

@adamdoe what do you mean paste? the only options should not be to either upload (screenshot 1) or edit (screenshot 2)

@joshlacey joshlacey force-pushed the feature/dev-8574-advanced-editor branch from a75a080 to 222e337 Compare July 9, 2024 15:56
@adamdoe
Copy link
Collaborator

adamdoe commented Jul 9, 2024

@adamdoe what do you mean paste? the only options should not be to either upload (screenshot 1) or edit (screenshot 2)

@joshlacey There is an option to edit the config. When I click on the pencil icon for the full config and replace it the previous config is still used.

@joshlacey
Copy link
Collaborator Author

@adamdoe what do you mean paste? the only options should not be to either upload (screenshot 1) or edit (screenshot 2)

@joshlacey There is an option to edit the config. When I click on the pencil icon for the full config and replace it the previous config is still used.

did you also hit apply?

@adamdoe
Copy link
Collaborator

adamdoe commented Jul 10, 2024

@adamdoe what do you mean paste? the only options should not be to either upload (screenshot 1) or edit (screenshot 2)

@joshlacey There is an option to edit the config. When I click on the pencil icon for the full config and replace it the previous config is still used.

did you also hit apply?

Correct here are the issues I'm seeing with steps:

Issue 1. Loading a config from the starting tile screen

  • Rebuild local
  • Run the editor
  • Upload a working chart config from the examples folder
  • Config does not populate if I return to step one
  • Config does not load data in screen two
  • Config asks to choose a visualization type if I skip to step 3

Issue 2. Loading a config from the json editor

  • Rebuild local
  • Run the editor > Choose an example config > Land on step 3
  • Scroll to Advanced Editor, on the top level edit the config
  • Upload an example working config from chart or map package that matches the initial tile selected
  • Hit Apply
  • I believe the prior config loads in at this point. When I checked the title properties I wasn't seeing a new title populated and the config was empty.

@joshlacey joshlacey force-pushed the feature/dev-8574-advanced-editor branch from 1c74576 to 081c5ed Compare July 11, 2024 20:41
@adamdoe adamdoe merged commit 56d002e into dev Jul 11, 2024
1 check passed
adamdoe added a commit that referenced this pull request Jul 11, 2024
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

2 participants