-
Notifications
You must be signed in to change notification settings - Fork 14
Implement Upsert Metric Modal for Experiments #2648
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
Implement Upsert Metric Modal for Experiments #2648
Conversation
…ision-point-modal
|
I have not looked at this very deeply yet, just got started on Friday afternoon. I already like this a lot more than the old modal, looks nice. We should have some research team members use this and see what notes they have. I think they'll be relieved to see it updated, this makes more sense to me at least. |
|
small thing, when editing I can submit without making changes. in other edit modals I think we usually don't allow submit until user enters something. |
|
I don't think we have employed 'speedbumps' in newer edit modals for 'are you sure you want to discard changes?' I know we have that in some places in old design. I think we may want that for certain forms if the user is entering enough info to be frustrated by an accidental exit. Which is to say, maybe this modal ought to have a speedbump, thoughts @zackcl @amurphy-cl @bcb37 |
|
I'd like to ask other research folks if the display of this information works for them. Maybe it does, I know it's basically what we have been doing all along. It's always looked like text soup to me. And the way the words line up particularly in a group-metrics row makes my brain automatically want to read each row left-to-right, where each row is an entity and the columns are properties. which isn't the way this reads obviously, a little jarring even knowing how this reads in columns. I don't know if we can go wrong labelling the crap out of everything metric related, metrics are complicated. The table deserves the same level of friendliness as the new modal. our #upgrade3.0 adoptees will appreciate it. @zackcl @amurphy-cl @bcb37
|
danoswaltCL
left a comment
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.
Left comments on some things that are missing, and some design questions for the team.
|
Closing due to a new PR #2756 that includes all the changes made in this PR. |







Resolves #2631