-
Notifications
You must be signed in to change notification settings - Fork 128
Fix empty button role #2557
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
base: main
Are you sure you want to change the base?
Fix empty button role #2557
Conversation
|
| * - `{button}`<text>`` => button links to `text`, shows `text`. | ||
| * - `{button}`text<label>`` => button links to `label`, shows `text`. | ||
| */ | ||
| const bodyTargetMatch = BODY_TARGET_PATTERN.exec(body); |
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.
Instead of two regexs, why not match on the standard pattern: text<link>, and then do the logic of figuring out what to display: if is empty, show text, if text is empty, show link.
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.
I.e., two groups, one for text, one for link, including triangle brackets. Then can check if link == <> and in that case render nothing for the link.
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.
I had split it into two just to be really explicit because the last REGEX I found pretty opaque, but then again I find all regexes opaque, so sounds good I will revert back to a one-liner :-)
|
OK this is now two PRs - one for myst-theme, I updated the body to have links etc. I think the failing test is that flaky DOI test |
I'm relying on CI to test this
Now when buttons don't have a link specified (e.g.
{button}`Text`) it emits aspanwith.buttonclass instead of a link button that is empty.Right now this won't be styled so this PR adds a style to it: