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

V4 proposed aligned paragraph tokens #3908

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

dbolack-ab
Copy link
Collaborator

Description

This implements the suggested syntax for V4 paragraph text alignment tokens from #2879

Related Issues or Discussions

#2879

QA Instructions, Screenshots, Recordings

  • insert :- in front of a paragraph. It should align left. You should see a class of mdParagraphJustifyLeft on the paragraph tag
  • insert -: in front of a paragraph. It should align right. You should see a class of mdParagraphJustifyRight on the paragraph tag
  • insert :-: in front of a paragraph. It should align center. You should see a class of mdParagraphJustifyCenter on the paragraph tag

Reviewer Checklist

@dbolack-ab dbolack-ab added 🔍 R0 - Needs first review 👀 PR ready but has not been reviewed 🚀 V4 Wait for V4 labels Nov 23, 2024
@calculuschild
Copy link
Member

As long as these syntaxes don't conflict with existing documents, we should be able to just add them to V3. I think this one should be pretty safe.

@5e-Cleric
Copy link
Member

i want to test this

@calculuschild
Copy link
Member

calculuschild commented Nov 25, 2024

Three thoughts:

First, the table syntax this borrows from allows any number of hyphens in the construction. Is it valuable to allow that here as well? For example:

"I am inevitable."
--------------: Michael Scott

Second, is it valuable to allow this syntax on the first line of a paragraph
by itself, to act as an optional familiar appearance more like the Table syntax?

This is a normal paragraph.

---------------------------:
This paragraph is going
to be right-aligned.

:--------------------------:
This paragraph is going
to be centered

This paragraph is unmarked,
and so defaults back to
left-aligned

Third, we should aim to have Markdown convert to pure HTML, so they do not rely upon CSS to function. In this case, we have the align HTML property we can use, the same way it is handled in Tables.

All this said, please don't start writing code for the first two unless we get a consensus, so you don't have to undo it later.

@ericscheid
Copy link
Collaborator

This paragraph is unmarked,
and so defaults back to
left-aligned

Technically, since that ¶ is unmarked it should default back to the default .. which in many themes might be left-aligned, but could also conceivably be full-aligned in some themes.

I suggest

:----------------
This para is marked to be 
explictly left aligned

This paragraph is unmarked,
and so is the default for the
current theme

@dbolack-ab
Copy link
Collaborator Author

Three thoughts:

First, the table syntax this borrows from allows any number of hyphens in the construction. Is it valuable to allow that here as well? For example:

I'm dubious, but I don't see any reason that there needs to be a fixed number unless we want to ascribe value based on the number of hyphens ( example - 1: use HTML Align 2: use class )

Second, is it valuable to allow this syntax on the first line of a paragraph by itself, to act as an optional familiar appearance more like the Table syntax?

I think it is better suited to being a lead-in paragraph-marker.

Third, we should aim to have Markdown convert to pure HTML, so they do not rely upon CSS to function. In this case, we have the align HTML property we can use, the same way it is handled in Tables.

I chose to use a class instead of the property so that users could overload the feature with additional styling. If we do it with the property, users lose that option and must {{}} it as-well.

@5e-Cleric
Copy link
Member

I chose to use a class instead of the property so that users could overload the feature with additional styling. If we do it with the property, users lose that option and must {{}} it as-well.

To be fair, centered text is something common to most if not all themes, i'm not sure of the value of the overriding possibility. Not against it.

@dbolack-ab
Copy link
Collaborator Author

I chose to use a class instead of the property so that users could overload the feature with additional styling. If we do it with the property, users lose that option and must {{}} it as-well.

To be fair, centered text is something common to most if not all themes, i'm not sure of the value of the overriding possibility. Not against it.

I'm guessing I explained that poorly? I dunno. The notion that a brew or theme could choose to override and extend what :-: did struck me as useful.

@Gazook89
Copy link
Collaborator

You could use the attribute selector [align=center] to apply styles rather than using a class

@dbolack-ab
Copy link
Collaborator Author

You could use the attribute selector [align=center] to apply styles rather than using a class

See above.

@calculuschild
Copy link
Member

calculuschild commented Dec 5, 2024

Markdown should translate to plain HTML wherever possible. This is a case where we don't need CSS at all, so we should not need to make this one a special exception to the rule.

The notion that a brew or theme could choose to override and extend what :-: did struck me as useful.

I agree, and people can still override and extend the functionality of :-: if they want, using the selector suggested by Gazook89. A CSS class is not needed to accomplish that.

// Change all `:-:` paragraphs to actually be left-aligned, with a black border and red text.
p[align=center] {
  text-align: left;    // CSS will override the align="center" attribute
  border: solid black;
  color: red;
}

Markdown tables also use the HTML align attribute for their syntax instead of CSS. It will be a more unified syntax if we follow suit there.

@5e-Cleric
Copy link
Member

I agree with calc here

@5e-Cleric
Copy link
Member

This has changed the behaviour of paragraphs:

image

The first two paragraphs are separated as two p elements with different alignment.

The second two paragraphs without the syntax are joined in the same p element.

The second behaviour is desired, we have two ways of separating them, a blank line or two spaces at the end, as the current behaviour stands, both are ignored if alignment is set.

@5e-Cleric 5e-Cleric added 🔍 R1 - Reviewed - Needs Discussion 💬 PR not okayed yet; needs reevaluation or closure and removed 🔍 R0 - Needs first review 👀 PR ready but has not been reviewed labels Dec 14, 2024
@dbolack-ab
Copy link
Collaborator Author

This has changed the behaviour of paragraphs:

I believe I have everything running correctly now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔍 R1 - Reviewed - Needs Discussion 💬 PR not okayed yet; needs reevaluation or closure 🚀 V4 Wait for V4
Projects
Status: Waiting for Calc's First Review
Development

Successfully merging this pull request may close these issues.

6 participants