Skip to content
This repository was archived by the owner on Dec 11, 2021. It is now read-only.

New Component: Notifications #161

Closed

Conversation

nashvail
Copy link
Contributor

No description provided.

@jquerybot
Copy link

Thank you for your pull request. It looks like this may be your first contribution to a jQuery Foundation project, if so we need you to sign our Contributor License Agreement (CLA).

📝 Please visit http://contribute.jquery.org/CLA/ to sign.

After you signed, the PR is checked again automatically after a minute. If there's still an issue, please reply here to let us know.


If you've already signed our CLA, it's possible your git author information doesn't match your CLA signature (both your name and email have to match), for more information, check the status of your CLA check.

@sfrisk
Copy link
Contributor

sfrisk commented May 25, 2016

Hey, there was an issue with your signature:

nashvail nishantve1@gmail.com: Name on signature (Nishant Verma, Nash Vail) doesn't match name on commit.

font-weight: map-get($notifications-element, font-weight);
padding: map-get($notifications-element, padding);
font-size: map-get($notifications-element, font-size);
background: bg($type);
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need to call functions to get this. Check out how we do it in buttons, just pass in the $notification -success/$notifications-default/etc variable into this mixin and map-get the needed variable from that. Will make it easier if additional notification colors ever get added, instead of having to add an additional else if statement in both of your function files

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

so results in:

background: map-get($type, background);

@nashvail nashvail force-pushed the notifications-initial branch from 22dea10 to 691a185 Compare June 8, 2016 06:35
@nashvail nashvail changed the title Notifications: Styling New Component: Notifications Jun 12, 2016
@sfrisk sfrisk added this to the Phase Two milestone Jun 14, 2016
@sfrisk sfrisk closed this Feb 14, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants