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

fix: Temporarily disable editing metric title #12384

Closed
wants to merge 1 commit into from

Conversation

kgabryje
Copy link
Member

@kgabryje kgabryje commented Jan 9, 2021

SUMMARY

Temporarily disable editing metric title until displaying custom title is supported.
Closes #12375
Closes #12379

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

image

TEST PLAN

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

CC: @junlincc @villebro

@kgabryje kgabryje closed this Jan 9, 2021
@junlincc junlincc added the explore:control Related to the controls panel of Explore label Jan 10, 2021
@ktmud
Copy link
Member

ktmud commented Jan 11, 2021

@kgabryje why is this closed? I think it's reasonable to disable custom label edits for Saved Metrics until we add support for adhoc overrides.

@kgabryje
Copy link
Member Author

@ktmud This PR disables edits for every metric as I misunderstood the requirement. Disabling edits just for saved metrics would require some more refactoring and we decided that we should wait for the overrides feature instead of introducing changes that would soon need a revert. However, if you think it's vital to disable edits for saved metrics until the feature is implemented, I could take it on again

@ktmud
Copy link
Member

ktmud commented Jan 11, 2021

I think it's worthwhile to do some refactoring and add a "overridable" flag or something as even if we enabled overrides for saved metrics some companies may not want that because of consistency. For example, at Airbnb, I've heard people asking the same metric should have the same name across all dashboards/charts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
explore:control Related to the controls panel of Explore size/L
Projects
None yet
3 participants