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

Prevent dropdown focus ring on click #2544

Merged
merged 8 commits into from
Oct 26, 2022

Conversation

mark-drastrup
Copy link
Contributor

Which issue does this PR close?

This PR closes #2477

What is the new behavior?

The dropdown won't get a focus ring on click

Does this PR introduce a breaking change?

  • Yes
  • No

Checklist:

The following tasks should be carried out in sequence in order to follow the process of contributing correctly.

Reminders

  • Make sure you have implemented tests following the guidelines in: "The good: Test".
  • Make sure you have updated the cookbook with examples and showcases (for bug fixes, enhancements & new components).

Review

  • Do a self-review.
  • Request that the changes are code-reviewed
  • Request that the changes are UX reviewed (only necessary if your PR introduces visual changes)

When the pull request has been approved it will be merged to develop by Team Kirby.

@mark-drastrup mark-drastrup changed the title Toggle 'clicked' class to prevent focus ring on click Prevent dropdown focus ring on click Oct 12, 2022
@github-actions github-actions bot temporarily deployed to pr-2477-dropdown-focus-state October 12, 2022 11:52 Inactive
@github-actions github-actions bot temporarily deployed to pr-2477-dropdown-focus-state October 12, 2022 11:55 Inactive
@github-actions github-actions bot temporarily deployed to pr-2477-dropdown-focus-state October 14, 2022 11:11 Inactive
@github-actions github-actions bot temporarily deployed to pr-2477-dropdown-focus-state October 21, 2022 08:21 Inactive
@github-actions github-actions bot temporarily deployed to pr-2477-dropdown-focus-state October 24, 2022 09:50 Inactive
@github-actions github-actions bot temporarily deployed to pr-2477-dropdown-focus-state October 25, 2022 11:05 Inactive
@mark-drastrup mark-drastrup merged commit c02a88f into develop Oct 26, 2022
@mark-drastrup mark-drastrup deleted the bug/2477-dropdown-focus-state branch October 26, 2022 05:37
@RasmusKjeldgaard RasmusKjeldgaard mentioned this pull request Oct 28, 2022
RasmusKjeldgaard added a commit that referenced this pull request Oct 28, 2022
* Fix no fabSheet close on item select

* Add unit-tests for fabSheet

* 💡 Add dummy TODO comment

* Work In Progress

* 🚧 Keep focusedIndex variable updated

* 🚧 Clean up a bit

* 🚧 Control arrow up/down depending on open/closed

* 🚧 Select item on Enter/Space keypress

* 🐛 Only select focused item on enter/space when open

* On opening move focus to selected index (if any)

* ✅ Extend test

Behavior has been changed: ArrowUp/ArrowDown no longer selects Item.
Instead they move focus to next item.

Updated test checks that items get focused. Extended with a key press on
Enter to trigger select Item.

When selecting an item it also programmatically gets focused.

* ✅ Update tests to match new behavior

- Key presses should not set selected item when dropdown is closed
- ArrowDown and ArrowUp opens the dropdown
- Space behaves like Enter (open and close dropdown)

* Update todo-comment

* ♿️ Make Home and End key press behave properly

Should not select item - only focus. Similar to arrow keys.
Should only apply when dropdown is open.

* ♿️ Focus on last item if key press ArrowUp

If no item is selected then move focus to last item if ArrowUp
Move focus to first item if ArrowDown.

* ⚗️ Try to remove focus outline on iOS

* 🚧 stash commit

* fixed top content directive

* removed cookbook example

* updates

* removed redundant style

* local navigation component

* update directive name

* rollback unwanted linting changes

* removed duplicate style value

* removed console log + redundant styles

* added console warn

* revert

* Refactor logic to focus items

* Remove the focusItem method

* Clean up code for focusing items

* Remove ToDo comments

* Utilize scrollIntoView to simplify scrolling logic

* Update cookbook example for custom item template

* Added Data Table examples and showcase

* Add data-table directive

* Add initial unittest to data-table directive

* rename directive from data-table to table

* Add option to show an alert before dismissing the modal

* Update cookbook

* Fix broken unit tests

* Create a separate method to handle showAlert

* Write unit tests to check if a bool or a function is passed to canDismiss

* Remove focused test

* Add initial styling of table

* Add showcase with icon button

* Reassign the showAlert method instead of creating a spy

* add mockdata to showcase + rmv constructor elem

* modified Camel case to Kebab case selectors

* Reorganize unit test in accordance with the AAA pattern

* Add accordian to example

* add colors from Kirby utils

* fixed top content spacing

* Remove unused CSS

* change row border-bottom & add global style vars

* Remove beforeEach and move the content into the it block, if there is only 1 it block

* Remove focused test

* fix index.ts exports

* add export DataTableModule

* solve conflicts + removed unsued imports

* Correct review changes from RKK

* Change naming convention of table components

* Write test to check if focusedIndex is set when selecting an item

* Write tests for arrow key behaviour

* Move css to relevant components

* Remove defined action-padding

* updates

* update showcase w/ disclaimer, API, CSS desc.

* refactor + review changes

* Update package.json

fix version bump

* line-height sass var

* Update core version

* Bump to version 7.1.1

* Update changelog

* update unittest imports

* corrected kirby-selectable-th class name

* add unittest for data table and table row

* rename tr to table-row for clarity

* implements onPush change detection
- onPush change detection on Table Component
- onPush change detection on Table Component
- import fix in Data table module

* unit tests + experimental examples module

* update table component unittest

* added cookbook affix examples

* remove unused imports

* added prefix / suffix slots to form-field.component.html

* remove fixed comments

* fix module scope

* Remove todo comment

* affix directive

* position affix correctly. Input field padding still wrong

* fix affix example to align with new syntax

* moved some shared form field variables to a new file from form-field-inputs.shared.scss. Trying to correctly calculate horizontal padding on input field iwth affix

* ResizeObserver for affix

* update divider widht and item spacing

* Refactor template to simplify calculations

* Leftalign code snippet for better presentation

* Remove affix heading from example page

* Use designtoken instead of css prop for padding

* Move ResizeObserver init to AfterContentInit hook

* Revert form-field-variables

* remove view encapsulation
- removed view encapsulation
- removed th selection class
- moved kirby-selectable-row to table main scss

* updated showcase card example subheading

* Simplify styling of affix and prefix/suffix container

* 🐛 Fix missing input element after content init

We should use the input content child in ngAfterContentInit to make
sure it is initialized.
The inputElement that was used is queried in ngAfterContentChecked,
and thus too late.

* Disable eslint for kirby-affix selector

* 📝 Add disclaimer with known limitations to docs

* ✅ Adjust tests to fit new DOM structure

* affixElements should never be undefined after content init

It will just be an empty array if no elements are queried.

* Increase readability of input padding calculation

* review changes

* remove onClickHeading

* Update cookbook example to include custom icon

* added some tests for affix on formfield spec

* increase wait time for formfield affix test

* Enforce affix text color as semi-dark

* ✅ Reuse existing test util and improve structure

* ✅ Remove flaky tests

* changed PR changes from RasmusKjeldgaard

* corrects kirby-table fixedLayout prop. in example

* updated styling comments

* bump designsystem version

* Set the Intersection Observer rootMargin to the height of the dropdown button

* Create settings.json in the .vscode folder

* Add update icon

* Update package.json

From code owner suggestion

Co-authored-by: Jesper Kaltoft Vind <git@jesperkaltoft.dk>

* Change position of tabbar badges (#2555)

* Change position of tabbar badges

* Adjust md badge vertical position

* Update libs/designsystem/src/lib/components/tabs/tab-button/tab-button.component.scss

Co-authored-by: Jesper Kaltoft Vind <git@jesperkaltoft.dk>

* Code Review Corrections

* Remove errornously merge foced test

Co-authored-by: Jesper Kaltoft Vind <git@jesperkaltoft.dk>

* Prevent dropdown focus ring on click (#2544)

* Toggle 'clicked' class to prevent focus ring on click

* Change comments to say 'dropdown' instead of 'modal'

* Refactor to use Hostbinding to dynamically add 'clicked' class

* Re-add method call, that was removed for test purposes

* Upgrade Chart.js to version 3.9.1 (#2548)

* Upgrade chart.js to v. 3.9.1

* Add type guard before accessing config.type

* Create helper type guard function

* Apply the type guard function where needed

* Bump charts.js version in nested package.json

* Remove skipLibCheck from tsconfig

* Release v7.2.1 (#2568)

* 7.2.1

* Update core version

* Update changelog

Co-authored-by: Michael T <micbittro@gmail.com>
Co-authored-by: RasmusTraeholt <rth@bankdata.dk>
Co-authored-by: Mark Drastrup <mark.drastrup@gmail.com>
Co-authored-by: bdusav <bdusav@bankdata.dk>
Co-authored-by: Sigurd P. von A. Vilstrup <SAV@bankdata.dk>
Co-authored-by: Sigurd <107846347+sigurdpvavilstrup@users.noreply.github.com>
Co-authored-by: Thomas Springborg <thsp@jyskebank.dk>
Co-authored-by: RasmusKjeldgaard <rkk@bankdata.dk>
Co-authored-by: tspringborg <thomas-springborg@hotmail.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.

[BUG] Dropdown focus state
3 participants