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(potentiometer): added keyboard reactions #16

Merged
merged 7 commits into from
Apr 13, 2020

Conversation

LironHazan
Copy link
Contributor

No description provided.

@urish
Copy link
Collaborator

urish commented Apr 11, 2020

Works like a charm! Two things are missing though:

  1. We should visually indicate when the element has a focus. The standard way to do it is using the outline CSS property
  2. Clicking on the element should focus it (currently the only way to focus it is by using the tab key)

@urish urish linked an issue Apr 11, 2020 that may be closed by this pull request
@LironHazan
Copy link
Contributor Author

Works like a charm! Two things are missing though:

  1. We should visually indicate when the element has a focus. The standard way to do it is using the outline CSS property
  2. Clicking on the element should focus it (currently the only way to focus it is by using the tab key)

@urish done, I used stroke instead of the outline for the rotating part of the knob (rect)

'#rotating'
);

this.inputEl?.addEventListener('focus', () => {
Copy link
Collaborator

@urish urish Apr 12, 2020

Choose a reason for hiding this comment

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

You can probably achieve the same with css, e.g. something like:

input:focus + svg #rotating or similar

also, in general - you can have lit-element manage the event handlers for you (by adding @focus and @blur attributes to the input)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks I actually tried input:focus + svg #rotating in one of my tries and it didn't work but maybe I selected it by class instead of id.. I'll check it out again :)
regarding the lit-element handlers totally right I wasn't thinking :)

@@ -81,6 +112,7 @@ export class PotentiometerElement extends LitElement {
/>
<rect x="5.4" y=".70" width="9.1" height="1.9" fill="#ccdae3" stroke-width=".15" />
<ellipse
filter="url(#outline)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about controlling this filter directly from CSS? then you don't really need the input event listeners and updating the stdDeviation from JS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

always better - will fix :)

@urish urish merged commit 4288d23 into wokwi:master Apr 13, 2020
@urish
Copy link
Collaborator

urish commented Apr 13, 2020

Released as version 0.13.1

karinchechik pushed a commit to karinchechik/wokwi-elements that referenced this pull request Apr 14, 2020
* fix(potentiometer): added keyboard reactions
* fix(potentiometer): click/focus glowing outline via css
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.

The potentiometer is not keyboard accessible
2 participants