Skip to content

Conversation

@eerison
Copy link
Contributor

@eerison eerison commented Mar 21, 2025

Description

I have added a box div into the Layout.astro, with this we can change the blog's layout to full screen if we want :)

e.g:

custom.css

@import "tailwindcss";

//overriding  global.css
body #box {
  @apply max-w-full;
}
Screencast.from.2025-03-21.18-32-10.webm

Types of changes

  • New Feature (non-breaking change which adds functionality)
  • Fix the issue on footer component

Checklist

  • I have read the Contributing Guide
  • I have added the necessary documentation (if appropriate)
  • Breaking Change (fix or feature that would cause existing functionality to not work as expected)

@eerison eerison changed the title [Feature]: Create main box to wrap blog structure [Feature]: Switch blog layout to fullscreen overriding global.css Mar 21, 2025
@satnaing
Copy link
Owner

I agree we could refactor some layout classes (especially max-w-4xl mx-auto) to reduce redundancy.
However, instead of current approach used in this PR, using other solutions like "utility-classes" would be better I guess.
Current approach has some issues with the layout as you can see in this video.

Screen.Recording.2025-03-22.at.5.04.51.PM.mov

@eerison
Copy link
Contributor Author

eerison commented Mar 22, 2025

Could you give me some example using "utility-classes" 😀

And I. Couldn't see the issue in your video 🥲

@satnaing
Copy link
Owner

Could you give me some example using "utility-classes" 😀

When you search max-w-3xl, there are a lot of components that uses max-w-3xl class. Also, max-w-3x is the main class that determines the layout of the website. So, we should combine those redundant classes. We can combine some redundant classes as well. (eg: max-w-3xl mx-auto). But we need to figure out a way to avoid breaking unexpected layouts.

TailwindCSS website ref

Screenshot 2025-03-22 at 5 35 01 PM

And I. Couldn't see the issue in your video 🥲

You can compare the footer with current deployed website.

@eerison eerison force-pushed the add-main-box branch 3 times, most recently from 3035aa0 to 7f2c535 Compare March 22, 2025 11:27
@eerison
Copy link
Contributor Author

eerison commented Mar 22, 2025

When you search max-w-3xl, there are a lot of components that uses max-w-3xl class. Also, max-w-3x is the main class that determines the layout of the website. So, we should combine those redundant classes. We can combine some redundant classes as well. (eg: max-w-3xl mx-auto).

Well I moved this to #box inside of global.css and removed the max-w-3xl mx-auto that you posted above.

Screenshot from 2025-03-22 13-09-59

You can compare the footer with current deployed website.

ohhh now I saw the diff, I am trying to find where I changed ...

Edit: I don't know why mt-auto or any other mt-x is not working on footer 😢

@eerison eerison force-pushed the add-main-box branch 2 times, most recently from 6e2ec54 to 6f8bcbe Compare March 22, 2025 12:08
@eerison eerison marked this pull request as draft March 24, 2025 11:27
It was added a box div wrapping the whole structure, it will be easier
to change the layout when it is needed.

Added layout for structure

astro-paper structure
@eerison eerison marked this pull request as ready for review March 24, 2025 13:02
@eerison
Copy link
Contributor Author

eerison commented Mar 24, 2025

Hey @satnaing

I fixed the footer issue. is it missing something else?

@satnaing
Copy link
Owner

satnaing commented Jun 7, 2025

Hi @eerison

After checking thoroughly throughout all the changes in this PR, I realized that we can approach this issue by only using a universal tailwind @utility class for max-width. But, we should be doing only the necessary part. I believe we should not do abstraction more than necessary. What do I mean by that? Let me explain.

We've used max-w-3xl throughout the entire app for layout.

`mx-auto flex max-w-3xl flex-col items-center justify-between sm:flex-row` // Header
`max-w-3xl mx-auto ${noPadding ? "px-0" : "px-4"}`; // Hr
`mx-auto prose mt-8 max-w-3xl` // PostDetails
`mx-auto w-full max-w-3xl px-4 pb-12` // PostDetails #main-content
`mx-auto flex max-w-3xl flex-1 items-center justify-center` // 404
`mx-auto max-w-3xl px-4` // Footer
`mx-auto w-full max-w-3xl px-4 pb-4` // Main
`prose mb-28 max-w-3xl prose-img:border-0` // AboutLayout
`mx-auto mt-8 mb-1 w-full max-w-3xl px-4`; // Breadcrumb;
`mx-auto flex w-full max-w-3xl items-center justify-start px-2`; // BackButton

We can get the desired result by changing max-w-3xl to something else (probably max-w-full).
Right now, we have to replace every single max-w-3xl with our desired max-w-*.

But we can do better by extracting max-w-* in all of these components and put the max-w-* inside a global @utility tailwind class.
In this way, we can easily change the overall layout of the app by changing that utility class. We don't have to replace every max-w-3xl.

So, there's another question. Why don't we extract other classes like mx-auto or px-4, which are usually used alongside max-w-3xl?

This is because with every additional extracted class (apart from max-w-3xl), we somehow have to overwrite the existing class.

For example, let's say we extract max-w-3xl and mx-auto to a utility class.

@utility awesome-class {
  @apply mx-auto max-w-full;
}

Now, we can replace mx-auto and max-w-full with awesome-class. The problem is that not every component that uses max-w-3xl has mx-auto class (eg, AboutLayout). So, we have to overwrite mx-* inside prose mb-28 max-w-3xl prose-img:border-0. Technically, we still can overwrite it, but I believe it's better to have only the necessary abstraction. In this way, if a user wants to make a simple modification, they don't have to check global files for extracted classes.

I'll make a PR to solve this issue, and you can leave your opinion on that as well.
That said, I'll close this PR for now. Thank you so much for the PR and our discussions.

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