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

Add CSS class support for box component #995

Merged
merged 4 commits into from
Sep 30, 2024
Merged

Conversation

richard-to
Copy link
Collaborator

Ref: #438

// Since component-renderer encompasses many different types of a components, we
// need to add a more specific selector to make these inline by default.
component-renderer:not([mesop-box]),
// The first component-renderer object is a box, but it needs to be inline.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does it need to be inline?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a great question. I wasn't quite able to figure that out. But when I looked at the generated markup, that first component-renderer box element does not have display block by default on main branch (which is definitely strange since in component renderer ts code, in the main branch, it should set the style to display block, at least that's what I would have thought).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I just remembered, I think it's because the first element isn't actually a box component, the root component is a special synthetic component that IIRC doesn't actually have a component type (the purpose of it is that sometimes you may have multiple top-level components under a page fn, so we want to treat it as a tree with the synthetic node). Maybe we should have it be a box component just to be more consistent? (I'm not sure if you noticed anything broke when switching from block to inline), but that can be a future change. this PR LGTM

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I noticed this because some things definitely broke when I changed it to display block. I think the primary issue occurred in the chat apps where the full height stuff wasn't working. So seems like something that could be adjusted.

@richard-to richard-to merged commit f60e66e into google:main Sep 30, 2024
6 checks passed
@pritam-dey3
Copy link

I would like to have a guide here with tailwind css example. I just started using mesop today, and feel lucky that this feature just dropped, since I know a little bit of tailwind-css and is much easier for me to arrange the layouts.

@richard-to
Copy link
Collaborator Author

Hi @pritam-dey3,

I'm planning to write a guide for it soon. Basically once I get the change for hosting static files on Mesop since that will make it easier to use tailwind.

What kind of info do you think would be helpful in the guide?

How to build the tailwind css for Mesop?
How tailwind css can be used for styling Mesop apps?

@richard-to richard-to deleted the box-class branch October 26, 2024 23:54
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.

3 participants