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 style to secondary action buttons #162

Closed
wants to merge 2 commits into from
Closed

Add style to secondary action buttons #162

wants to merge 2 commits into from

Conversation

guimspace
Copy link

This PR adds styling to buttons with class "secondary" to whenever they are secondary actions. For example, in a form with buttons Save, Reset, and Cancel, Save is the primary action whereas Reset and Cancel are secondary.

  • I didn't add the style to input[type="submit"] because (usually) they are the primary action.
  • I first tried to make a secondary action with a gray background, but it didn't work out well and got mixed up with the disabled buttons. So I simply made the background transparent.
  • The disabled mode of the secondary buttons don't look so great, so perhaps you can ignore the second commit and use the existing style to both primary and secondary actions.

I tested the changes with theses buttons from the Demo page:

<p>
	<input type="submit" value="<input type=submit>">
	<input class="secondary" type="submit" value="<input type=submit>">
</p>
<p>
	<input type="button" value="<input type=button>">
	<input class="secondary" type="button" value="<input type=button>">
</p>
<p>
	<input type="reset" value="<input type=reset>">
	<input class="secondary" type="reset" value="<input type=reset>">
</p>
<p>
	<input type="submit" value="<input disabled>" disabled>
	<input class="secondary" type="submit" value="<input disabled>" disabled>
</p>
<p>
	<button type="submit">&lt;button type=submit&gt;</button>
	<button class="secondary" type="submit">&lt;button type=submit&gt;</button>
</p>
<p>
	<button type="button">&lt;button type=button&gt;</button>
	<button class="secondary" type="button">&lt;button type=button&gt;</button>
</p>
<p>
	<button type="reset">&lt;button type=reset&gt;</button>
	<button class="secondary" type="reset">&lt;button type=reset&gt;</button>
</p>
<p>
	<button type="button" disabled>&lt;button disabled&gt;</button>
	<button class="secondary" type="button" disabled>&lt;button disabled&gt;</button>
</p>
<p>
	<a href="#forms__action" class="button">&lt;a class=button&gt;</a>
	<a href="#forms__action" class="button secondary">&lt;a class=button&gt;</a>
</p>
<p>
	<a href="#forms__action" class="button" aria-disabled="true">&lt;a class=button aria-disabled=true&gt;</a>
	<a href="#forms__action" class="button secondary" aria-disabled="true">&lt;a class=button aria-disabled=true&gt;</a>
</p>

Signed-off-by: Guilherme Tadashi Maeoka <guim.space@gmail.com>
Signed-off-by: Guilherme Tadashi Maeoka <guim.space@gmail.com>
@netlify
Copy link

netlify bot commented Aug 29, 2023

Deploy Preview for simplecss-cdn ready!

Name Link
🔨 Latest commit 4e1ee02
🔍 Latest deploy log https://app.netlify.com/sites/simplecss-cdn/deploys/64eda2cb60af470008384170
😎 Deploy Preview https://deploy-preview-162--simplecss-cdn.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@kevquirk
Copy link
Owner

This feels like something that's firmly in the custom CSS space, and probably shouldn't be included in the upstream project. I'm very reluctant to add additional classes. @lkhrs what do you think?

@lkhrs
Copy link
Collaborator

lkhrs commented Aug 29, 2023

I agree with Kev, we want to minimize additional classes.

@lkhrs lkhrs closed this Aug 29, 2023
@guimspace
Copy link
Author

Hey, thank you for the feedback, guys! This PR really felt like pushing the edge when I submitted it, but needed to share the idea anyways.

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

3 participants