-
Notifications
You must be signed in to change notification settings - Fork 1
feat: Accessibility for layout and repo explore bar #207
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
Conversation
@@ -16,7 +17,7 @@ | |||
<i class="bi bi-question-circle-fill text-primary me-1" aria-hidden="true"></i> | |||
<span data-modal-target="title"><%= t('.header_title') %></span> | |||
</h5> | |||
<small class="text-muted" data-modal-target="subtitle" style="margin-top: -3px;">This action cannot be undone.</small> | |||
<small class="text-muted" data-modal-target="subtitle" style="margin-top: -3px;"><%= t('.subtitle_disclaimer_text') %></small> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say this is subtitle_default_text
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
input.focus() | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will make it more generic like this:
import { Controller } from '@hotwired/stimulus'
export default class extends Controller {
static values = {
event: String,
elementId: String
}
connect() {
if (this.eventValue) {
this.element.addEventListener(this.eventValue, this.focusInput)
}
}
disconnect() {
if (this.eventValue) {
this.element.removeEventListener(this.eventValue, this.focusInput)
}
}
focusInput = () => {
if (!this.elementIdValue) return
const target = document.getElementById(this.elementIdValue)
if (target) target.focus()
}
}
For a BS collapsable item, it can listen to an event that BS fires:
data-controller="focus-on-event"
data-focus-on-event-event-value="shown.bs.collapse"
data-focus-on-event-element-id-value="id of element"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
a44fe8f
to
ba3c8f9
Compare
data-controller="focus-on-event" | ||
data-action="keydown.enter->focus-on-event#focusInput click->focus-on-event#focusInput" | ||
data-focus-on-event-event-value="shown.bs.collapse" | ||
data-focus-on-event-element-id-value="input_repo_url" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is to use the event or the data-action, but not both.
For components with events, like BS collapse, it can be used to ensure the collapse action has completed.
for other items, the data-action will be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have refactored the code to add the controller on the event receiver instead of the event triggered. Now data-action is not required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
fixes: #156