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

Nav Bar || User Dropdown Menu #8

Closed
vikasrohit opened this issue Mar 3, 2016 · 9 comments
Closed

Nav Bar || User Dropdown Menu #8

vikasrohit opened this issue Mar 3, 2016 · 9 comments

Comments

@vikasrohit
Copy link

This component contains links that are needed for a logged in user.

Component tree

  • user-dropdown-menu
    • dropdown-menu-header
      • user-image
      • username
      • dropdown-arrow
    • dropdown-menu-list(.user-menu-items-list)
      • std-list-item(.user-menu-item)

Requirements:

  • Create a branch off of master and make a PR to master
  • Should support non logged in view as rendered in the design specks
  • Should support logged in view as rendered in the design specks
  • Reuse the dropdown-menu component
  • Should be responsive to support the mobile layout
@reynard2007
Copy link
Contributor

Taking this

@reynard2007
Copy link
Contributor

May I clarify the following:

  1. The parent (whole) navbar component is on flex display so this will be a flex-item
  2. Will the login state change while on the same page will trigger a page change? I'm thinking if I am to handle the login state change in the component or just initialize it as either logged in or not.
  3. The user avatar will also use the purple placeholder?
  4. I see that this uses the std-list-item. Is that something I have to wait for or can I proceed with implementing this I don't think this is necessary since there is no icon for the list items on this component. In that case, can I just use .user-menu-item as class name to avoid conflicts?

@vikasrohit
Copy link
Author

Go ahead. 👍

1 Ideally, component (user dropdown) should be independent of its parent (nav bar in this case). It should only behave differently based on the context provide by the parent i.e. it should adjust its width based on space provided by the parent. For mobile (width < 768px), we can always assume this component to reduce its header just to the user avatar and omit the username as depicted in the live specks.
2 Good question. I think for the sake of simplicity and speeding up the development, you can just let the component behave based on the log in state. This state would be changed by external page (e.g. login page or /logout page).
3 Yes, you can. You can also use your profile pic as well. :)
4 Good catch. Yes, you can avoid using std-list-item for now. We can handle that in final fixes.

@reynard2007
Copy link
Contributor

Thanks, for answering my queries. Another thing: the codebase uses CamelCase in class names of primary components. Do we follow that or use the dash format (.user-dropdown-menu)?

@vikasrohit
Copy link
Author

@reynard2007 Basic idea is to have component names in title case (e.g. Panel, StandardListItem etc) while css class names in dash format (e.g. panel-header, panel-body). Only exception to dash format class name is the main css class of the component e.g. FileUploader, Checkbox etc. We are aware that Panel component's main css class is not using title case as of now, we have to fix that.

@reynard2007
Copy link
Contributor

What are we to do if we have this on minimal width state of the user dropdown. Curently it renders like this on my machine:

image

@vikasrohit
Copy link
Author

I think it is fine. On mobile devices, we would show the dropdown content as inline element in the DOM through the configuration in dropdown component.
Minor changes, as final fixes, can be done even after merging the PR.

@vikasrohit
Copy link
Author

@reynard2007 There are conflicts in PR#18, please resolve them to enable us to review the code. Further, it would be really great if you can separate out the PRs for #4 and #8. This would allow us to better track changes per task.

@vikasrohit
Copy link
Author

Closing task. Thanks @reynard2007 for your hard work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants