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

Blog post CMS #120

Merged
merged 13 commits into from
Jan 10, 2024
Merged

Blog post CMS #120

merged 13 commits into from
Jan 10, 2024

Conversation

5andu
Copy link
Contributor

@5andu 5andu commented Dec 30, 2023

demo.mp4

@5andu 5andu changed the title Blog post cms Blog post CMS Jan 2, 2024
@ryanckulp
Copy link
Owner

@5andu THANK YOU! a couple small requests before we merge:

  1. we shouldn't need the trix.css file, this gets included by the library
  2. there is some competing Trix styling inside both app/assets/stylesheets/application.tailwind.css and app/assets/stylesheets/actiontext.css. let's combine these into 1 file, either actiontext.css or a new blog.css for even more clarity. the application.tailwind.css file should just have tailwind overrides + extensions
  3. tiny detail, but inside blog_posts/show the <% meta title: "#{@blog_post.title} %> (+ description) doesn't need "quoted" interpolation, just raw Ruby

@5andu
Copy link
Contributor Author

5andu commented Jan 8, 2024

@ryanckulp

  1. I'm not using the default trix.css file, I have added custom styling in the file for the editor & content, removing it breaks the styles of the editor, as well as the custom spacing I added in content. Is it ok to keep the trix.css file?
    with@2x
    without@2x
    CleanShot 2024-01-08 at 22 55 04@2x
    CleanShot 2024-01-08 at 22 54 48@2x

  2. I deleted the rich text related styles from app/assets/stylesheets/application.tailwind.css as they were unnecessary.

  3. Fixed for the title & description, good catch thanks!

@ryanckulp
Copy link
Owner

@5andu there are definitely a few custom styles, but a lot of it is duplicate from the source. we want to only host overrides and extensions, not the source. this lets us update to our Trix version without concerns.

new styling example

original source
image

our trix.css

image

(cool)

duplication examples

original source
image

our trix.css (here)

image

original source

image

our trix.css

image

(notice how these look a bit different, but the variables are the same. and variable references are better than hard-coded)

image

next steps

we could just merge, then maybe i can help remove dupe code. my strategy would be to combine all the Trix source CSS into 1 file, then do a git diff between them, or use a web UI tool to visualize diffs.

lemme know what you think!

@5andu
Copy link
Contributor Author

5andu commented Jan 10, 2024

@ryanckulp I've deleted unnecessary classes and made the rich text more simple and functional. Here's a recap of my changes:

trix.improvements.mp4

@ryanckulp
Copy link
Owner

excellent work, thanks for the second push. merging.

@ryanckulp ryanckulp merged commit d39489e into ryanckulp:master Jan 10, 2024
2 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.

2 participants