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.

Sign up for GitHub

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

Jump to bottom

feat(): allow metrics visibility #1322

Merged
merged 6 commits into from
Nov 8, 2023
Merged

feat(): allow metrics visibility #1322

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ffa4a4b
fix(): add metrics-coming-soon permission
jordantsanz Nov 6, 2023
324e517
feat(): add optional visibility to IMetricDisplayConfig
jordantsanz Nov 7, 2023
bbf7f97
Merge branch 'master' of github.com:Esri/hub.js into f/6633-metrics-view
jordantsanz Nov 7, 2023
4168dd5
refactor(): change visibility to enum
jordantsanz Nov 7, 2023
aeb779f
refactor(): change IMetricDisplayConfig interface
jordantsanz Nov 8, 2023
04b187d
fix(): update typing
jordantsanz Nov 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions packages/common/src/core/types/Metrics.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,22 +216,37 @@ export enum ExpressionRelationships {
}

/**
* Configuration for how to show a metric in the ui
* Configuration for how to show a metric in the ui. These are just values that are important to how to
* display the metric; values that are configured in the editor should be added to IMetricEditorValues instead.
*/
export interface IMetricDisplayConfig {
export interface IMetricDisplayConfig extends IMetricEditorValues {
metricId: string;
displayType: string;
[key: string]: any;
visibility?: MetricVisibility;
Copy link
Contributor

@juliannemarik juliannemarik Nov 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe you already have plans to do this, but I'm wondering if we want to expose other explicit (optional) display config properties on this interface.

Actually, I'm taking a closer look at this file and wondering if IMetricDisplayConfig should extend IMetricEditorValues rather than the other way around (which is what is currently happening)? IMetricEditorValues should be properties that come directly out of the editor, whereas IMetricDisplayConfig could include other additional properties (like metricId, visibility, etc). Am I confused?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think you've got it right -- I'll refactor it to the other way around.

I think we could refactor it to have the optional display config properties -- I'll start looking into that in the create/edit PRs

}

/**
* Editor values expected when editing a metric
* Types of states of visibility a metric can be in
* featured, visible, or hidden
*/
export interface IMetricEditorValues extends IMetricDisplayConfig {
export enum MetricVisibility {
visible = "visible",
hidden = "hidden",
featured = "featured",
}

/**
* Editor values expected when editing a metric. These are options that are set in the metric editor that
* help build the IMetricDisplayConfig.
*/
export interface IMetricEditorValues {
/** the main value of the metric */
value?: string | number;
/** all values related to constructing a service-query metric */
dynamicMetric?: IDynamicMetricValues;

// TODO: enumerate all editor values here
[key: string]: any;
}

/**
Expand Down