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

fix(platform): menu component library review fixes #1711

Merged
merged 1 commit into from
Dec 12, 2019

Conversation

kavya-b
Copy link
Contributor

@kavya-b kavya-b commented Dec 6, 2019

Please provide a link to the associated issue.

#1710

Please provide a brief summary of this pull request.

Address PR #1563 comments by Frank: #1563 (comment)
Additionally,

  1. Need to use fd-icon instead of span classes
  2. Using dropdown popover with button instead of deprecated fd-dropdown-control

Please check whether the PR fulfills the following requirements

Documentation checklist:

@kavya-b kavya-b added the platform platform label Dec 6, 2019
@netlify
Copy link

netlify bot commented Dec 6, 2019

Deploy preview for fundamental-ngx ready!

Built with commit 4dc47f2

https://deploy-preview-1711--fundamental-ngx.netlify.com

@kavya-b kavya-b changed the title Platform menu code change fix(platform): menu component library review fixes Dec 9, 2019
@kavya-b kavya-b self-assigned this Dec 11, 2019
1. Using fd-icon instead of span classes
2. fixing item offset calculation when icons are present in an edge case
3. adding longer if conditions into their own methods
4. added a new type FdpItem instead of MenuItem|MenuGroup and changed examples to reflect the same
5. removed !==undefined comparisons
6. Using dropdown popover with button instead of deprecated fd-dropdown-control
@KevinOkamoto KevinOkamoto added this to Approved by Reviewer in Platform Development via automation Dec 12, 2019
@kavya-b kavya-b merged commit cbd4949 into master Dec 12, 2019
@kavya-b kavya-b deleted the platform-menu-code-change branch December 12, 2019 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform platform
Projects
No open projects
Platform Development
  
Approved by Reviewer
Development

Successfully merging this pull request may close these issues.

None yet

2 participants