Skip to content

Conversation

@samglover
Copy link

This PR adds ids and classes for easier CSS/JS selectors.

Added ids:

  • #memo-list
  • #memo-comments
  • #home-sidebar

Added classes:

  • .memo
    • .tagged-[tag] for all tags
    • .memo-header
    • .memo-content
  • .memo-detail-sidebar

Closes #4481

@samglover samglover requested a review from boojack as a code owner March 11, 2025 18:47
@samglover
Copy link
Author

I'm getting a formatting error, but I'm not sure there's actually a problem. And part of the error relates to existing code that I didn't feel comfortable modifying.

@johnnyjoygh
Copy link
Collaborator

I'm getting a formatting error, but I'm not sure there's actually a problem. And part of the error relates to existing code that I didn't feel comfortable modifying.

Try run pnpm lint --fix in your dev environment to solve these format errors.

@johnnyjoygh
Copy link
Collaborator

@samglover We should ideally standardize to a specific attribute, such as data-classname, instead of mixing in id and class. e.g.,

<div data-classname="header-sidebar"></div>

@samglover
Copy link
Author

samglover commented Mar 12, 2025

I'm not sure I understand why you'd want to add a data attribute instead of just adding ids and classes. Just makes it harder to write selectors for CSS. .memo-header is regular CSS and easier to write than [data-classname="memo-header"].

But it's not my project so feel free to do it the way you'd prefer. I just want to be able to add custom CSS, and currently it's not very easy to do that, so when you invited me to submit a PR, I did.

@johnnyjoygh
Copy link
Collaborator

@samglover Since memos uses Tailwind, the className is already occupied by things like px-2, so it shouldn't be a good place to save custom class. Additionally, IDs should not conflict, so a better approach is to place it in a data- attribute, and to avoid breaking the existing code style.

This is an open-source project, and I think that code maintainability is more important than the the convenience of code for custom styles.

@samglover
Copy link
Author

Fair enough. I just want to be able to actually use the CSS and JS customizations. Right now they're not very useful because it's not possible to reliably target memos or memo content elements.

If someone wants to take over this PR and change it to use data attributes instead of ids and classes I can work with that.

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.

Ids & classes to make it easier to customize Memos with CSS and Javascript

3 participants