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

Allow Hashtags in Timeline Titles, Sections, and Events #5399

Merged
merged 2 commits into from
Aug 20, 2024

Conversation

FutzMonitor
Copy link
Contributor

📑 Summary

This pull request removes the hashtag from the title, section, and event regex so that hashtags can be used in their text content. The period element were not included because of an inconsistency. Whereas you can put a hashtag anywhere in the string for a title, section, and event, the period element begins a line and thus is counted as comment.

These changes were made to allow for hashtags in the timeline diagram and to address the inconsistency between percent comments and hashtag comments. This way functionality isn't outright broken with a new comment notation ## versus #.

Resolves #5356 .

📏 Design Decisions

I simply removed the hashtag symbol from the exclusion clause regex for the title, section, and event. I made sure to add test coverage to the timeline test suite and run pnpm test to ensure that it was still working.

📋 Tasks

Make sure you

1. Updated the regex for title, section, and event elements to allow hashtags.
2. Period elements remain unchanged.
Changes to timeline.spec.js
1. Added test coverage to ensure that the title, section, and event elements allow hashtags without error.
Copy link

netlify bot commented Mar 20, 2024

Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit d1a3a84
🔍 Latest deploy log https://app.netlify.com/sites/mermaid-js/deploys/660864333b1c540008668f85
😎 Deploy Preview https://deploy-preview-5399--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.

@FutzMonitor FutzMonitor changed the title Changes to timeline.jison Allow Hashtags in Timeline Titles, Sections, and Events Mar 21, 2024
Copy link

codecov bot commented Mar 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 5.74%. Comparing base (8e95c4d) to head (d1a3a84).
Report is 683 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #5399   +/-   ##
=======================================
  Coverage     5.74%   5.74%           
=======================================
  Files          277     276    -1     
  Lines        41899   41888   -11     
  Branches       489     514   +25     
=======================================
  Hits          2407    2407           
+ Misses       39492   39481   -11     
Flag Coverage Δ
e2e ?
unit 5.74% <ø> (+<0.01%) ⬆️

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

see 1 file with indirect coverage changes

@sidharthv96 sidharthv96 added this pull request to the merge queue Aug 20, 2024
Merged via the queue into mermaid-js:develop with commit 210f8c2 Aug 20, 2024
19 checks passed
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.

Timeline Diagram Comment Inconsistencies
2 participants