-
Notifications
You must be signed in to change notification settings - Fork 14
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
UI changes in Add Metrics page #1600
Conversation
…In ADD METRIC json-editor, added context list and removed highlighted blue line
@zackcl @danoswaltCL We are updating the UI of our table component, which includes a hierarchical tree structure. Initially, we used drop-down icons for all nodes. Now, changes made here to create difference between leaf node and expandable are: Expandable nodes use the drop-down icon and leaf nodes use bullet points (•). Which design option do we prefer for representing leaf nodes in our table component: |
@KD1712 Can you please add the reference image? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found the following things in the UI:
-
The chips are not spaced as they should be in tags chips on experiment page.
-
The bullet points are square, can you make them round bullets? @zackcl What is your opinion on this?
-
The context chips when clicked, wont trigger search operation as we have on experiments page.
-
Can the alignment of the bullet point and arrow for different rows be more aligned in a straight line?
FYI @zackcl is out until July 1st. Tagging @amurphy-cl to decide if someone else should have input on these. |
Thanks Husni, @danoswaltCL can you have a look? |
...de/src/app/features/dashboard/profile/components/modals/add-metrics/add-metrics.component.ts
Outdated
Show resolved
Hide resolved
Adding one more thing, the context from dropdown on json editor is not saving metrics successfully, I guess that hook with backend is not implemented yet, and context might be needed in the JSON itself. I am using the below JSON to add new metrics:
|
…t from select menu is attached to the Metric JSON on saving
|
…into 1295-appContext-in-metrics
* Crud operation and store related changes for fetch feature flag paginated * added service changes and shifted some logic to selectors
* WIP: Feature flag admin endpoints * update feature flag service tests * review changes * merge dev into feature/feature-flag-admin-api * review changes --------- Co-authored-by: danoswaltCL <[email protected]>
@KD1712 Thanks for adding a padding between the text and delete icon. I noticed one more thing, the delete icon is black and large in size, but we use a small and grey shade at all other places. Can you pick the css from design stepper page or profile page? |
…into 1295-appContext-in-metrics
@KD1712 I checked again, all looks good now. But, the delete button in the delete metrics modal is not enabled on correct metric path name. It is was working earlier and works in dev branch as well. Something is changed in this PR. Can you please check? |
...projects/upgrade/src/app/core/experiment-design-stepper/experiment-design-stepper.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KD1712 Have few comments, please check.
...projects/upgrade/src/app/core/experiment-design-stepper/experiment-design-stepper.service.ts
Outdated
Show resolved
Hide resolved
.../projects/upgrade/src/app/features/dashboard/profile/components/metrics/metrics.component.ts
Show resolved
Hide resolved
.../projects/upgrade/src/app/features/dashboard/profile/components/metrics/metrics.component.ts
Show resolved
Hide resolved
import { JsonEditorOptions, JsonEditorComponent } from 'ang-jsoneditor'; | ||
import { MatDialogRef } from '@angular/material/dialog'; | ||
import { AnalysisService } from '../../../../../../core/analysis/analysis.service'; | ||
import { Subscription } from 'rxjs'; | ||
// import { MetricUnit } from '../../../../../../core/analysis/store/analysis.models'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this commented import
Replaced ID column with Context column, added context filter option. In ADD METRIC json-editor, added context list and removed highlighted blue line