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 the mdim edit hack #4104

Merged
merged 1 commit into from
Nov 14, 2024
Merged

🔨 fix the mdim edit hack #4104

merged 1 commit into from
Nov 14, 2024

Conversation

danyx23
Copy link
Contributor

@danyx23 danyx23 commented Nov 4, 2024

Fixes the hack to make the internal edit link in the share menu of grapher work for Mdims (#3987)

Copy link
Contributor Author

danyx23 commented Nov 4, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @danyx23 and the rest of your teammates on Graphite Graphite

@danyx23 danyx23 marked this pull request as ready for review November 4, 2024 09:55
@owidbot
Copy link
Contributor

owidbot commented Nov 4, 2024

Quick links (staging server):

Site Admin Wizard

Login: ssh owid@staging-site-fix-mdim-edit-hack

SVG tester:

Number of differences (default views): 0 ✅
Number of differences (all views): 0 ✅

Edited: 2024-11-04 10:12:26 UTC
Execution time: 1.17 seconds

@danyx23 danyx23 force-pushed the fix-mdim-edit-hack branch 2 times, most recently from 9fa24fa to 4cf6efb Compare November 4, 2024 17:06
@danyx23 danyx23 force-pushed the fix-mdim-edit-hack branch 2 times, most recently from cd7bcf8 to 76b50b2 Compare November 4, 2024 17:16
@danyx23 danyx23 requested a review from marcelgerber November 4, 2024 17:24
Comment on lines +283 to +297
variables?.length === 1
? `variables/${variables[0]}/config`
: undefined
Copy link
Member

Choose a reason for hiding this comment

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

Hm, this will mean that if there is more than a single y variable, the URL will still be wrong...

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 - do you think it's worthwhile turning editUrl into an object so we can communicate "do not show any edit link"?

Copy link
Contributor Author

danyx23 commented Nov 14, 2024

Merge activity

  • Nov 14, 12:59 PM EST: A user started a stack merge that includes this pull request via Graphite.
  • Nov 14, 12:59 PM EST: A user merged this pull request with Graphite.

@danyx23 danyx23 merged commit c8058b1 into master Nov 14, 2024
21 of 24 checks passed
@danyx23 danyx23 deleted the fix-mdim-edit-hack branch November 14, 2024 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mdims: Remove the hack to open the indicator chart admin when clicking on edit in the share menu
3 participants