Skip to content

Migrate chart view editors to plugin architecture - #361

Merged
benthongtiang merged 3 commits into
masterfrom
ben/chart
Jul 28, 2026
Merged

Migrate chart view editors to plugin architecture#361
benthongtiang merged 3 commits into
masterfrom
ben/chart

Conversation

@benthongtiang

@benthongtiang benthongtiang commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Migrate chart view editors to plugin architecture

@benthongtiang benthongtiang added the minor Tag Pull Requests to trigger a minor version update label Jul 8, 2026

@johnny-hausman johnny-hausman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just a little reworking on the hard links and good to go.


export default function FNAbviewchartEditor({ AB, ABViewEditorPlugin }) {
const FABViewContainer =
require("../../../rootPages/Designer/editors/views/ABViewContainer").default;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

there is an FABViewContainer available in the passed in resources that is the /rootPages/Designer/properties/views/ABViewContainer.

instead of making a hard link like this:

  1. update the ABDesigner/index.js to include the editors/views/ABViewContainer in the ABDesignResources variable. Make it FABViewContainerEditor to distinguish it from the current one.
  2. reference that in the passed in resources.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, I've updated it.

const ABViewContainer = FABViewContainer(AB);
const BASE_ID = "interface_editor_viewchart";

const ABAbviewchartEditor = class ABAbviewchartEditor extends ABViewContainer {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we remove the constant here and just define the class? They are both named the same thing.

ABViewPropertiesPlugin,
}) {
const FABViewContainer =
require("../../../rootPages/Designer/properties/views/ABViewContainer").default;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is already available in the passed in resources as FABViewContainer

const ABViewContainer = FABViewContainer(AB);
const uiConfig = AB.Config.uiSettings();
const L = ABViewContainer.L();
const ABAbviewchartProperties = class ABAbviewchartProperties extends ABViewContainer {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we not have 2 things named the same thing? lets just use the class definition and not the constant.

@johnny-hausman johnny-hausman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Lookin good!

@benthongtiang
benthongtiang merged commit 5e51cb4 into master Jul 28, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor Tag Pull Requests to trigger a minor version update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants