Skip to content

Add event button classname is not applied #264

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

Merged
merged 3 commits into from
Apr 18, 2025

Conversation

The-Best-Codes
Copy link
Contributor

@The-Best-Codes The-Best-Codes commented Apr 18, 2025

Before

@AndreaGriffiths11 Made the events grid so much prettier 🤩

However, there are a couple of things I noticed:

  1. The icon on button links is black (this is an issue with all the button links).
  2. The margin between the text and button seems to be less than the text line-height (a picky little thing).

image

In the process of messing with this, I discovered a third thing. There are classes that are not applied because, while the className prop was passed to the button link, the button link doesn't accept a className prop and so it is never applied.

After

With all fixes applied, the new section looks like this:

image

The button is white due to changes made in PR #260 by @AndreaGriffiths11. See the specific diff here:
https://github.com/github/maintainermonth/pull/260/files#diff-da275896c80ebad87819e008c15b36a54697e45dfe79e5d4f61f205365b66c6dR27
I could revert this if you think it would look better the other way.

Other button links look the same as before but with the correct icon color. Example:

Before:
image

After:
image


I'm sure you can think of plenty of changes to make, please give feedback! ❤️

I also have a question for the maintainers. What is the contributing process? Should contributors always create an issue before making a PR, or can they just make a PR (like I just did 😳)? Thanks!

@karasowles
Copy link
Collaborator

Love the update, thank you!!

@karasowles karasowles merged commit 4daaf94 into github:main Apr 18, 2025
1 check passed
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