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

feat: redesign dashboard UI #118

Open
wants to merge 1 commit into
base: redesign
Choose a base branch
from
Open

feat: redesign dashboard UI #118

wants to merge 1 commit into from

Conversation

laxyapahuja
Copy link

@laxyapahuja laxyapahuja commented Jan 19, 2020

Issue Fix

Fixes #102

Screenshots

Description

The addition of bottom navigation bar will be done in different PRs as discussed with a mentor.

  • Run the unit tests with ./gradlew check to make sure you didn't break anything

  • If you have multiple commits please combine them into one commit by squashing them.

@rchtgpt
Copy link
Member

rchtgpt commented Jan 19, 2020

hey @laxyapahuja, I think what you have done is not correct because according to the mockups which we are currently working on, a Persistent BottomSheetDialog should be used instead of a CardView to display recent transactions. You can check the prototype video of the mockups here

@laxyapahuja
Copy link
Author

laxyapahuja commented Jan 19, 2020

hey @laxyapahuja, I think what you have done is not correct because according to the mockups which we are currently working on, a Persistent BottomSheetDialog should be used instead of a CardView to display recent transactions. You can check the prototype video of the mockups here

hey, i just checked out BottomSheetDialog and it doesn't exactly look like what is in the actual mockup. i think it'll be better if we used CardView and just add some code to drag it up. also either way, the task only mentions to "redesign" so i think the functionality can be added in another pull request.

@rchtgpt
Copy link
Member

rchtgpt commented Jan 19, 2020

hey, i just checked out BottomSheetDialog and it doesn't exactly look like what is in the actual mockup. i think it'll be better if we used CardView and just add some code to drag it up. also either way, the task only mentions to "redesign" so i think the functionality can be added in another pull request.

The mockup which I made was actually influenced from a BottomSheetDialog and in my opinion, it is a part of design itself.
@luckyman20 what do you think?

@luckyman20
Copy link
Collaborator

luckyman20 commented Jan 20, 2020

@laxyapahuja You have to add this dragging functionality as this is part of frontend only.

Also, 'Direct' doesn't seems appropriate here, use 'Self' instead.

@laxyapahuja
Copy link
Author

updated :D

@rchtgpt
Copy link
Member

rchtgpt commented Jan 21, 2020

according to the mockup, you have to add a white outline to the 'Receive' button

@luckyman20
Copy link
Collaborator

You have to shift notification bell icon too. Also, the green thing on each card is not looking good, either remove it or colour it in violet color. Do not forget to update screenshot/video recording ;)

@laxyapahuja
Copy link
Author

You have to shift notification bell icon too. Also, the green thing on each card is not looking good, either remove it or colour it in violet color. Do not forget to update screenshot/video recording ;)

updated :)

@rchtgpt
Copy link
Member

rchtgpt commented Jan 21, 2020

@laxyapahuja i just compared your GIF with the actual mockup, i feel like the cards do not look as they are in mockups.

@laxyapahuja
Copy link
Author

Yeah, the mock-up has very less padding which makes it look congested.

@openMF openMF deleted a comment from punwai Jan 22, 2020
@rchtgpt
Copy link
Member

rchtgpt commented Jan 23, 2020

@laxyapahuja, you can avoid importing bell.png file and instead use the bell icon which is already present in the app with the name ic_notifications.xml

@rchtgpt
Copy link
Member

rchtgpt commented Jan 23, 2020

Adding a blank line at the end of each file is considered a good practice, so I suggest you can add them.

@laxyapahuja
Copy link
Author

updated :)

app/build.gradle Outdated Show resolved Hide resolved
@garvit984
Copy link
Collaborator

Swipe Refresh can be applied for the whole fragment. Please upload a screenshot for switching between different tabs. Remove the toolbar. Bell icon is not consistent with the design.

@garvit984
Copy link
Collaborator

@laxyapahuja There are inconsistencies in some devices in relation to the height this bottom sheet can rise.

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.

4 participants