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(popper): arrow option & docs consistency across components #875

Merged
merged 4 commits into from
Oct 27, 2023

Conversation

connerblanton
Copy link
Contributor

πŸ”— Linked issue

#873

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Adding the arrow option to all components that use popper. This provides consistent popper options across these components. Docs for each of these components have been updated to demonstrate how to use the popper prop.

Resolves #873

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@vercel
Copy link

vercel bot commented Oct 26, 2023

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Updated (UTC)
ui βœ… Ready (Inspect) Visit Preview Oct 27, 2023 0:22am

@benjamincanac
Copy link
Member

What would you think of filling the arrow background based on the border of the component? It would be more visible in my opinion:
CleanShot 2023-10-27 at 00 46 58@2x

@connerblanton
Copy link
Contributor Author

I think that's a great idea! It will still be customizable if anyone wants to change it, but for the default I think this will look better

@connerblanton
Copy link
Contributor Author

@benjamincanac seeing differences for the dark mode of these poppers.

selectMenu - dark:bg-gray-800 dark:ring-gray-700
dropdown - dark:bg-gray-800 dark:ring-gray-700
tooltip - dark:bg-gray-900 dark:ring-gray-800
popover - dark:bg-gray-900 dark:ring-gray-800
contextMenu - dark:bg-gray-900 dark:ring-gray-800

They are all the same for light mode - bg-white ring-gray-200

Do we want to make all those dark mode classes consistent? If so, which one would you prefer?

@connerblanton
Copy link
Contributor Author

I think I would vote to make them consistent and go with the dark:bg-gray-800 dark:ring-gray-700 classes for all of them. Those are needed to show active states on selectMenu and dropdown. If we chose the dark:bg-gray-900 then we would actually need to take those up to dark:bg-gray-950 to account for hover and active states on the selectMenu and dropdown, which seems like a bigger change than we would want for this PR.

So with that in mind, there are two options we can go with:

Leave the background and ring classes as they are and make the arrow background match the ring color. That would give us these results
Screenshot 2023-10-26 at 6 47 20 PM

Or we make them all the same dark:bg-gray-800 dark:ring-gray-700 which would give use these results
Screenshot 2023-10-26 at 6 45 52 PM

@benjamincanac
Copy link
Member

I think it should adapt to the border of the component, we don't have to make them all the same 😊

@benjamincanac benjamincanac changed the title feat: Consistent Popper Options & Docs feat(popper): props & docs consistency across components Oct 27, 2023
@benjamincanac benjamincanac changed the title feat(popper): props & docs consistency across components feat(popper): options (arrow) & docs consistency across components Oct 27, 2023
@benjamincanac benjamincanac changed the title feat(popper): options (arrow) & docs consistency across components feat(popper): arrow option & docs consistency across components Oct 27, 2023
@benjamincanac benjamincanac merged commit f785ecd into nuxt:dev Oct 27, 2023
2 checks passed
@benjamincanac
Copy link
Member

Thank you 😊

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.

Consistent Popper Options
2 participants