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

Rework Notification System + CSS Modules (WIP) #297

Merged
merged 53 commits into from
Nov 5, 2017
Merged

Conversation

evgenyboxer
Copy link
Contributor

@evgenyboxer evgenyboxer commented Nov 3, 2017

What current issue(s) from Trello/Github does this address?
Improving notification system + Introduce CSS Modules.

What problem does this PR solve?
The current implementation was lacking types, was not dismissible by default, so you always had to call clearTransaction, could not set a position, and was lacking styling.
This PR brings a new system:

  1. Four types/levels of notification (success/error/info/warning)
  2. Dismissible by default
  3. Close button
  4. Can have custom styling
  5. Positioning (Top/Bottom)
  6. Animated

Also, I added CSS Modules, and it works with CSS/SCSS. I made it possible to still load "global" files by having a .global.scss as a filename. I did some initial work of moving a lot of the styles into their respective component, created a variables.scss file, and more.

How did you solve this problem?

  1. Implemented a new reducer - notification that has functions for each notification - showSuccessNotification(..) showInfoNotification(..) etc.
  2. Removed everything that was associated with the older way of showing a notification.
  3. Enabled CSS Modules.
  4. Split the large main.scss file into smaller ones, and put them next to their component.
  5. Created some more "primitive" component for better reusability, such as <Button> and <HomeButtonLink>

How did you make sure your solution works?
Currently tests are broken, and I'm working on fixing that. Other than that, the wallet is pretty solid.

I also added some UX goodies to the login pages, like having titles, so they would look similar to other pages + padding fixes.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.2%) to 42.881% when pulling 1019fb8 on rework-notif-system into 3cd991d on dev.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.4%) to 42.991% when pulling 426bff0 on rework-notif-system into 3cd991d on dev.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.4%) to 42.991% when pulling 6d7cf18 on rework-notif-system into 3cd991d on dev.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.4%) to 42.991% when pulling 61b568b on rework-notif-system into 3cd991d on dev.

@evgenyboxer evgenyboxer merged commit e3e610c into dev Nov 5, 2017
@evgenyboxer evgenyboxer deleted the rework-notif-system branch November 5, 2017 00:31
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.

3 participants