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

Fixed xyChart.html link and renamed xyChart.md to xychart.md #4986

Closed
wants to merge 1 commit into from

Conversation

shaikshahid98
Copy link

@shaikshahid98 shaikshahid98 commented Oct 26, 2023

📑 Summary

Brief description about the content of your PR.
Fixed xyChart.html link and renamed xyChart.md to xychart.md

Resolves #4984

📏 Design Decisions

Describe the way your implementation works or what design decisions you made if applicable.
Just changed the html link and renamed file as mentioned.

📋 Tasks

Make sure you

@netlify
Copy link

netlify bot commented Oct 26, 2023

Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit 185fb56
🔍 Latest deploy log https://app.netlify.com/sites/mermaid-js/deploys/653a45364fa78200087db7f2
😎 Deploy Preview https://deploy-preview-4986--mermaid-js.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

Merging #4986 (185fb56) into develop (5f117fc) will decrease coverage by 0.46%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4986      +/-   ##
===========================================
- Coverage    80.17%   79.71%   -0.46%     
===========================================
  Files          164      164              
  Lines        13820    13820              
  Branches       693      693              
===========================================
- Hits         11080    11017      -63     
- Misses        2591     2654      +63     
  Partials       149      149              
Flag Coverage Δ
e2e 85.66% <ø> (-0.57%) ⬇️
unit 42.94% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 7 files with indirect coverage changes

Copy link
Contributor

@nirname nirname left a comment

Choose a reason for hiding this comment

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

I don't see any renaming although your request description states that. And we don't need one.

We only changed link in demos pages, which is ok, but not sufficient. Demo pages are not documentation website. In order to change link on the documentation website you need to edit vitepress config

{ text: 'XYChart 🔥', link: '/syntax/xychart' },

@aloisklink
Copy link
Member

Hi @shaikshahid98, thanks for your contribution. Unfortunately, we went with PR #5014 since that fixed the issue and the bug was a bit important.

We only changed link in demos pages, which is ok, but not sufficient

Actually, I believe the link here points to demos/xychart.html, which is actually already correct.

But, it could make sense to rename this file to demos/xyChart.html to match the capitalization of xyChart.md to make things consistent. If you want to, @shaikshahid98, feel free to open a new PR for this!

@aloisklink aloisklink closed this Nov 8, 2023
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.

Docs - Syntax - XYChart 404 Page not found
3 participants