Skip to content
This repository has been archived by the owner on Jan 4, 2021. It is now read-only.

Import of Biohazard's work #74

Open
wants to merge 37 commits into
base: master
Choose a base branch
from
Open

Conversation

LalitMaganti
Copy link
Contributor

This is an import of the excellent work done by @BioHaZard1 on materializing various parts of the app.

My opinion on the changes and this PR:

The Good:

  • The nav drawer looks awesome
  • Thread items also look very cool
  • News items look better too

The Neutral:

  • BLOCKER: post/thread selection states are broken - this needs to be fixed before merge
  • News items could be better - I have tweaked the design from BioHaZard's design
  • Posts - I've made these full bleed tiles to match threads
  • Thread/Post items - I've tweaked various design elements here to make stuff look a bit nicer

The Not So Good

  • Use of dividers in forums - this contradicts MD principles - I've removed the dividers - would appreciate feedback on this
  • Use of non-keyline padding/margins - also removed this since it just makes things inconsistent
  • BLOCKER: Create dialogs - the post and thread dialogs look kinda weird with the change in colour - this should be changed before merge
  • BLOCKER: I hate the press twice to exit - I hate it on every app I encounter this behaviour. I'm strongly in favour of removing this.

Thanks again for the awesome work BioHaZard!

Tagging: @rwestergren, @ddrager

BioHaZard1 and others added 30 commits October 17, 2014 09:00
* Material Design specifically states that in this usecase, dividers are unecessary
  distractions from the important content.
* Removing the divider reduces the noise and makes it look better too IMHO
* also remove instances of padding/margins not conforming to keyline
* also remove unused layouts
@ddrager
Copy link
Contributor

ddrager commented Jul 27, 2015

@tilal6991 I'm wondering what the progress is on this pull request, would love to have these changes merged in!

@LalitMaganti
Copy link
Contributor Author

I was waiting for @BioHaZard1 to comment but then totally forgot about this stuff. It should still work TBH.

@BioHaZard1
Copy link
Contributor

Yeah sorry about that, completely forgot to comment. I think I've fixed everything you wanted done before the merge though.

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.

3 participants