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

question: Add class name to all sections #87

Open
Brixy opened this issue Apr 22, 2024 · 3 comments
Open

question: Add class name to all sections #87

Brixy opened this issue Apr 22, 2024 · 3 comments

Comments

@Brixy
Copy link

Brixy commented Apr 22, 2024

Hi all,

This is just a question. Sorry if this is not the right place.

Adding class names to all sections using a filter is no problem.

This is my current solution. But is there a more elegant/suggested way of doing so (apart from ternary if notation)?

return {
	section: (el) => {
		if (typeof (el.autoAttributes) === 'object') {
			el.autoAttributes.class = 'my-class';
		} else {
			el.attributes.class = 'my-class';
		}
	},
};

Thanks a lot!

@jgm
Copy link
Owner

jgm commented Apr 22, 2024

I see that this is painful. We could perhaps always add an empty object for attributes instead of using null? Or export a function that allows setting an attribute value?

@matklad any thoughts?

@matklad
Copy link
Contributor

matklad commented Apr 22, 2024

Yeah, in my blog I have the following snippet (which is still on a very old version of djot):

function add_class(node: AstNode, cls: string) {
  node.attributes = node.attributes || {};
  const attr = node.attributes["class"];
  node.attributes["class"] = attr ? `${attr} ${cls}` : cls;
}

It deals with both attribute object being empty, and with the fact that adding a class requires string manipulation.

We could perhaps always add an empty object for attributes instead of using null?

I think this would be a clear win, yes. This means that we erase the syntactic distinction between no attributes and an empty attribute set on the semantic level, but I think that's correct. Djot's AST guarantees semantics, it doesn't guarantee round-tripping through original data.

Though, given than we now distinguist between auto attributes and normal attributes, maybe round-trippability is a goal now?

Or export a function that allows setting an attribute value?

For manipulating attributes, I think we don't need to do that, if we always set them to an empty object. Manipulating class list though additionally requires string splitting, and for that I think a bunch of helper functions would be nice.

If djot AST objects were proper classes then el.addClass|hasClass|removeClass would have been no-brainers. But our AST objects are just JSON without any methods, and that feels like the right design. So, addClass should live as a free functions somewhere. Which is perhaps fine? Funamentally, these helpers are just easy to maintain shortcut, so adding&maintaining them is low-cost, even if we later discover some better way to solve the problem.

@Brixy
Copy link
Author

Brixy commented Apr 23, 2024

Thank you very much, guys. Your proposed solutions would simplify such use cases.

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

No branches or pull requests

3 participants