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

feat: add the browser mockup component from README.md list #2147

Merged
merged 5 commits into from Jul 20, 2023

Conversation

malkiii
Copy link
Contributor

@malkiii malkiii commented Jul 18, 2023

Added the browser component that I saw in README list, I made this mockup responsive and simple as possible.

@saadeghi saadeghi self-assigned this Jul 19, 2023
@saadeghi
Copy link
Owner

saadeghi commented Jul 20, 2023

Thank you very much.

Can you please apply these changes:

  1. The icon at the right side, I don't think it is necessary. We have 3 circles at the left so it looks like an OS (MacOS) window, but the icon at the right would have no purpose and it may create confusions.
    To remove the icon (:after) and keep the address bar in the middle you can use inline-grid grid-cols-3 for the .toolbar

  2. In the docs (/components/mockup-browser/) you used <div> for the code example but you used <a> in the previews. Please Use <div> there too. Users can use any tag but I don't think it should be an actual clickable link in the docs preview.

  3. Please don't force a custom border-radius for the .mockup-browser .toolbar .input. Let it adapt the current border radius of the theme (For example it should be sharp border in cyberpunk theme and it should be rounded-full in cupcake theme. Otherwise the it won't fit the theme)

@malkiii
Copy link
Contributor Author

malkiii commented Jul 20, 2023

the changes that are applied:

  • Remove the right icon in the mockup
  • Using <div> instead of <a> tag in the docs
  • Remove the border-radius in input

I found mx-auto is much better than inline-grid grid-cols-3 since the input width will be limited to 1fr

please let me know if there are other changes you want me to do.

@saadeghi
Copy link
Owner

Perfect. Thank you!

@saadeghi saadeghi merged commit b58282e into saadeghi:master Jul 20, 2023
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.

None yet

2 participants