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: Side Navigation styles markup adaptation #2490

Merged
merged 23 commits into from Jun 2, 2020

Conversation

JKMarkowski
Copy link
Contributor

@JKMarkowski JKMarkowski commented May 8, 2020

Please provide a link to the associated issue.

Please provide a brief summary of this pull request.

Please check whether the PR fulfills the following requirements

Documentation checklist:

@JKMarkowski JKMarkowski requested a review from a team May 8, 2020 13:21
@JKMarkowski JKMarkowski added this to In progress in Development via automation May 8, 2020
@JKMarkowski JKMarkowski added this to the Sprint 36 - Plovdiv milestone May 8, 2020
@netlify
Copy link

netlify bot commented May 8, 2020

Deploy preview for fundamental-ngx ready!

Built with commit 5dfc147

https://deploy-preview-2490--fundamental-ngx.netlify.app

@JKMarkowski JKMarkowski force-pushed the fix/side-navigation-changes branch 3 times, most recently from ed601de to 7c5c784 Compare May 11, 2020 17:09
@JKMarkowski JKMarkowski changed the title WIP feat: Side Navigation styles markup adaptation feat: Side Navigation styles markup adaptation May 11, 2020
@mikerodonnell89
Copy link
Member

List item with icon has a different focus outline
Screen Shot 2020-05-11 at 3 22 20 PM

@mikerodonnell89
Copy link
Member

Issue with the border on selected items
Screen Shot 2020-05-11 at 3 34 06 PM

@mikerodonnell89
Copy link
Member

Items should not have these outlines I think
Screen Shot 2020-05-11 at 3 35 02 PM
Screen Shot 2020-05-11 at 3 34 43 PM

Development automation moved this from In progress to Review in progress May 11, 2020
Copy link
Member

@mikerodonnell89 mikerodonnell89 left a comment

Choose a reason for hiding this comment

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

Few issues, see conversation

@JKMarkowski
Copy link
Contributor Author

JKMarkowski commented May 12, 2020

@mikerodonnell89 It looks completely different than local examples. After rebasing, there was wrong version of fundamental-styles. It's fixed.

Copy link
Member

@mikerodonnell89 mikerodonnell89 left a comment

Choose a reason for hiding this comment

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

have fdescribe on a few tests

@mikerodonnell89
Copy link
Member

Pushed a commit to this branch addressing this issue: #2392

Copy link
Contributor

@salarenko salarenko left a comment

Choose a reason for hiding this comment

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

  • ArrowLeft / ArrowRight navigates up and down
  • No support for Spacebar / Enter selection
  • This is not using KeyUtil

@JKMarkowski JKMarkowski changed the title feat: Side Navigation styles markup adaptation WIP feat: Side Navigation styles markup adaptation May 19, 2020
@JKMarkowski
Copy link
Contributor Author

@InnaAtanasova I can't reproduce it. Maybe you had some old cached preview
image

Development automation moved this from Review in progress to Reviewer approved Jun 1, 2020
@JKMarkowski JKMarkowski merged commit ecd6c25 into master Jun 2, 2020
Development automation moved this from Reviewer approved to Done Jun 2, 2020
@JKMarkowski JKMarkowski deleted the fix/side-navigation-changes branch June 2, 2020 07:44
fkolar pushed a commit that referenced this pull request Jun 5, 2020
* feat: Side Navigation styles markup adaptation

* Add tests

* Fix comments

* Update package-lock.json

* Add comments, fix rtl on side nav condensed popover

* remove focused tests

* Add PR comments

* remove focus outline overrides

* add PR comments

* Add transparent button

* Remove non-direct imports, add tests for aggregated directives

* Remove public accessors, remove unused imports, move properties

* fix tests

* update styles

* Remove unused parameter on host listener

* Fix focus on children elements for parent item

* dd selection on click, remove arrowright/left handling

* remove focused test

* Fix lint

* fix tests for side nav keyboard support

* Fix programatically changed example

* fix selected state in object generated examples

Co-authored-by: Mike O'Donnell <mikerodonnell89@users.noreply.github.com>
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.

None yet

7 participants