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

#347 Update Sidebar + Navbar to match designs #390

Merged

Conversation

sandrina-p
Copy link
Contributor

@sandrina-p sandrina-p commented Apr 28, 2018

Following and Closes #347

I've updated Sidebar and Navbar layout to match the design on #347.

  • Now the layout is fluid (100% window width), but the Dashboard itself has a max-width, so isn't too large.
  • Sidebar "is-active" logic isn't working, so any tab is never active, but the design for the class is done. BTW, how can I debug easily a vue component? On react it's easy as { console.log('x')} but on Vue I couldn't make it work, neither using breakpoints... What's the trick?
  • I've noticed the designs don't have Pay Group link, so to not erase it, I've kept it on navbar until we figure out a better way.
  • The "Create Group" link was added to homepage "Welcome to GroupIncome" when logged for the first time.

Some parts of the layout don't match exactly the design (typography sizes / colors). It only uses bulma classes for now. I believe we can still iterate a lot on the navigation (sidebar + navbar) and elements hierarchy, but let's keep that for another iteration when we have #387 merged. I prefer to have small but fast iterations, what do you think?

image

@sandrina-p sandrina-p force-pushed the feat/347-update-sidebar-design branch 2 times, most recently from 2a9481c to 9af2a5c Compare April 28, 2018 18:07
@sandrina-p sandrina-p changed the title #347 Update Sidebar + Navbar to match designs Update Sidebar + Navbar to match designs on #347 Apr 28, 2018
@sandrina-p sandrina-p changed the title Update Sidebar + Navbar to match designs on #347 #347 Update Sidebar + Navbar to match designs Apr 28, 2018
@sandrina-p sandrina-p force-pushed the feat/347-update-sidebar-design branch from 9af2a5c to b1f7927 Compare April 28, 2018 18:14
@taoeffect
Copy link
Member

Sidebar "is-active" logic isn't working, so any tab is never active, but the design for the class is done

Should this perhaps be fixed before merging?

BTW, how can I debug easily a vue component? On react it's easy as { console.log('x')} but on Vue I couldn't make it work, neither using breakpoints... What's the trick?

Sorry, don't understand. What are you trying to do? We can do a video chat if you'd like, and/or ask @hubudibu for help.

but let's keep that for another iteration when we have #387 merged. I prefer to have small but fast iterations, what do you think?

Should I review and merge this PR before #387 or after #387?

@sandrina-p
Copy link
Contributor Author

Both PR are independent, but if it this one goes after it gets cleaner.

@sandrina-p sandrina-p force-pushed the feat/347-update-sidebar-design branch from b1f7927 to ea74b76 Compare April 29, 2018 01:28
@hubudibu
Copy link
Contributor

I'm also not sure what you miss from debugging, but from the top of my head

the template does'nt get translated to JS like the JSX in react, so you can't write plain js in the template tags, that is true. maybe if you give an example on what you would console.log I could help more

@sandrina-p sandrina-p assigned sandrina-p and unassigned sandrina-p Apr 30, 2018
Copy link
Member

@taoeffect taoeffect left a comment

Choose a reason for hiding this comment

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

merge conflict needs fixing

@sandrina-p sandrina-p force-pushed the feat/347-update-sidebar-design branch from 8b3736c to c9d8f77 Compare May 2, 2018 20:28
@sandrina-p
Copy link
Contributor Author

Sidebar "is-active" logic isn't working, so any tab is never active, but the design for the class is done
Should this perhaps be fixed before merging?

Fixed @taoeffect 😊

@taoeffect taoeffect merged commit 906247b into okTurtles:master May 3, 2018
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