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

Convert hero section styling to tailwind #185

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

devilkiller-ag
Copy link
Contributor

Summary of Changes

This PR converts the styling of the hero section to Tailwind CSS.

Related Issue

#179 #177 #182

Checklist

  • I have read and followed the project's contribution guidelines, including code style and commit message conventions.
  • My code is well-documented, and I've updated relevant documentation.
  • I have added or updated test cases to ensure the code's functionality.
  • I have tested these changes on my local environment.
  • All tests pass, and there are no new linting errors.
  • I have reviewed and proofread my code and the changes.
  • The branch is up-to-date with the base branch.

Reviewer(s)

@Abhijay007

@devilkiller-ag devilkiller-ag marked this pull request as draft February 9, 2024 09:29
@devilkiller-ag
Copy link
Contributor Author

Hey @Abhijay007 Can you check this out?

@Abhijay007
Copy link
Collaborator

Hey @Abhijay007 Can you check this out?

sure

Copy link
Collaborator

@Abhijay007 Abhijay007 left a comment

Choose a reason for hiding this comment

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

Hi @devilkiller-ag, Thanks for investing your time to add Tailwind support to the Uptane site. There are just a few things I want to mention.

The hero section is nearly complete, but there are some minor issues regarding styling and responsiveness. While it looks quite good overall, there are some alignment issues (which I believe can be resolved with adjustments in margins and padding), and it appears slightly different on mobile devices. So, we need to ensure it's properly optimized for mobile as well.

It doesn't need to be perfect this time, but it would be great if we could save some work for later, So it would be great if you could fix it to some extent; otherwise, it looks good to me. Once you've made these adjustments, I'll merge it.

your code :

image

current site :

image

your mobile code :

image

Current code on Mobile device:

image

src/components/Header/index.jsx Outdated Show resolved Hide resolved
@devilkiller-ag
Copy link
Contributor Author

@Abhijay007 Seems like I forgot to check the mobile styling. I will do that today.

@Abhijay007
Copy link
Collaborator

@Abhijay007 Seems like I forgot to check the mobile styling. I will do that today.

No problem, take your time 👍

@devilkiller-ag
Copy link
Contributor Author

@Abhijay007 Can you check it now?

@Abhijay007 Abhijay007 marked this pull request as ready for review February 14, 2024 08:18
Copy link
Collaborator

@Abhijay007 Abhijay007 left a comment

Choose a reason for hiding this comment

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

Looks better now. Just the text subtext color is black in the hero section (change it to white) and remove the import styles from './styles.module.css'; as it is no longer needed.

image

@Abhijay007
Copy link
Collaborator

Abhijay007 commented Feb 14, 2024

Also, if we look closer: there are some resolution changes as well, can you please look into this as well?

current code :

image

your code :

image

same in the docs section :

see styles for the search button :

image

image

In the docs points and number bullets are also not visible :

image

current code :

image

@devilkiller-ag
Copy link
Contributor Author

Hi @Abhijay007, sorry for the long leave. I was busy with my college work. I tried searching the classes which are controlling the layout padding and search button styles. But I didn't get them any place. I am not sure where Docusaurus keeps it. Do you have any clue?

@Abhijay007
Copy link
Collaborator

Hi @Abhijay007, sorry for the long leave. I was busy with my college work. I tried searching the classes which are controlling the layout padding and search button styles. But I didn't get them any place. I am not sure where Docusaurus keeps it. Do you have any clue?

Hi @devilkiller-ag, I believe you can likely find the styles for this in "node_modules@docusaurus\theme-classic" within the node_modules directory. However, I'd like to mention that changing these styles may not necessarily address the sorting issue we're currently facing.

I'm considering a different approach. Is it possible to let Docusaurus render the custom CSS from there, where it is currently rendering, while concurrently using Tailwind for the components without disrupting the styles? I believe this approach could help resolve these issues, allowing us to utilize Tailwind and custom CSS without overriding the styles for prebuilt components unless necessary.

I have previously shared examples from other projects, such as Medusa.js and Supabase, where they seem to be successfully using multiple CSS files concurrently:

These examples seem to be employing a similar strategy, with multiple CSS files working concurrently. Therefore, I believe this could be a more effective approach to our situation, so can you please look into this?

@devilkiller-ag
Copy link
Contributor Author

Hey @Abhijay007, I got your point! I noticed that Medusa.js Docs is using three files for their styling:

  • _docusaurus.css: This CSS file includes tailwind styling definitions for classes defined by docusaurus
  • _variables.css: This stores the style variables
  • custom.css: for storing their custom styling for various components

They use this to define their custom CSS in docusaurus.config.js (as we do):

theme: {
     customCss: require.resolve("./src/css/custom.css"),
},

I think we should try overwriting the classes defined by docusaurus. However, I am not able to find how the connect _docusaurus.css with the overall doc? Is the name _docusaurus.css important to do this? I need to investigate a little more on this.

@Abhijay007
Copy link
Collaborator

Hi @devilkiller-ag any update on this ?

@devilkiller-ag
Copy link
Contributor Author

devilkiller-ag commented Mar 10, 2024

Hi @Abhijay007, earlier I re-structured CSS like that done in Medusa and rectified many styling. However, to solve the search icon size and sidebar width, I messed up with many docusaurus classes and doc search inifima variables. I tried to search variables used for font size in node modules and the internet got no working way to change the font size of the text inside the search icons and sidebar width. Also in this process, I messed up with all the headers. 🥲 I tried to fix them today but failed. I will retry tomorrow. Sorry for ghosting this issue, I will speed up the work now.

@Abhijay007
Copy link
Collaborator

Hi @Abhijay007, earlier I re-structured CSS like that done in Medusa and rectified many styling. However, to solve the search icon size and sidebar width, I messed up with many docusaurus classes and doc search inifima variables. I tried to search variables used for font size in node modules and the internet got no working way to change the font size of the text inside the search icons and sidebar width. Also in this process, I messed up with all the headers. 🥲 I tried to fix them today but failed. I will retry tomorrow. Sorry for ghosting this issue, I will speed up the work now.

No problem, @devilkiller-ag. Take your time. Also, I just want to mention that you don't need to be concerned about changing or revamping the whole site with Tailwind or using ShadCN UI for now. We just want the "Components" to be converted, so it's fine if Docusaurus utilizes the pre-added custom CSS and we can use Tailwind or ShadCN UI for the components concurrently.

I think you might need to rethink the approach you are following now; it might not require that much digging in. Let me know if you need any assistance regarding this. Also, don't stress too much on this. If achievable, then only invest time in this; otherwise, we will figure out something else. However, I do think that implementing the UI library just for the components might not be that difficult. We just need to focus on changing the approach a bit.

I will try to look into this whenever I get some time. Till then, you can investigate further if it's feasible. Thanks for putting your time into this.

@Abhijay007
Copy link
Collaborator

@devilkiller-ag are you still working on this PR ?

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