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

Accessiblity #1331

Merged
merged 2 commits into from
Aug 15, 2024
Merged

Accessiblity #1331

merged 2 commits into from
Aug 15, 2024

Conversation

pavpanchekha
Copy link
Collaborator

No description provided.

@pavpanchekha pavpanchekha requested a review from chrishtr August 15, 2024 13:39
@@ -324,11 +324,13 @@ class BlockLayout:
# ...
px_size = float(node.style["font-size"][:-2])
size = dpx(px_size * 0.75, self.zoom)
# ...
Copy link
Collaborator Author

@pavpanchekha pavpanchekha Aug 15, 2024

Choose a reason for hiding this comment

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

It's not the last line in the method

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not critical IMO

@@ -1482,10 +1486,10 @@ class LineLayout:
```

For the [focus example](examples/example14-focus.html), the focus outline
of an `<a>` element becomes red, as in Figure 6.
of an `<a>` element becomes thicker and red, as in Figure 6.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The red isn't visible in the book, but "thicker" is and they'll hopefully trust us on "red"

@@ -1625,7 +1629,6 @@ class AccessibilityNode:
def __init__(self, node):
self.node = node
self.children = []
self.text = ""
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not used until later.

Comment on lines -1825 to +1831
python3 -m pip install gtts
python3 -m pip install playsound
``` {.sh}
python3 -m pip install gtts
python3 -m pip install playsound
```
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should not be an example style.

Comment on lines -2443 to +2448
`hit_test` function I'm calling is the same one we wrote
[earlier in Section 7.3](chrome.md#click-handling) to handle clicks, except
of course that it's searching a different tree:
`hit_test` function recurses over the accessibility tree:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is not similar! An earlier draft of the book used similar code, but now the A11y hit-test method is recursive while layout hit-test isn't.

@@ -141,7 +141,7 @@ Zoom

Let's start with the simplest accessibility problem: text on the
screen that is too small to read. It's a problem many of us will face
sooner or later, and possibly the most common user disability issue.
sooner or later, and is possibly the most common user disability issue.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not critical IMO

@@ -279,8 +278,10 @@ class DocumentLayout:
``` {.python}
class Tab:
def render(self):
# ...
if self.needs_layout:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not critical IMO


def input(self, node):
# ...
# ...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not critical IMO

@@ -337,6 +340,7 @@ class InputLayout:
# ...
px_size = float(self.node.style["font-size"][:-2])
size = dpx(px_size * 0.75, self.zoom)
# ...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not critical IMO

@@ -1199,7 +1204,7 @@ def paint_outline(node, cmds, rect, zoom):
cmds.append(DrawOutline(rect, "black", 1))
```

Set this flag in a new `focus_element` method that we'll now use
Set this `is_focused` flag in a new `focus_element` method that we'll now use
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not critical IMO

@@ -1415,7 +1420,7 @@ Unknown pseudoclasses simply never match anything.
The focused element can now be styled. But ideally we'd also be able to
customize the focus outline itself and not just the element. That can be done
by adding support for the CSS [`outline` property][outline], which looks like
this to show a 3px red outline:[^outline-syntax]
this (for a 3-pixel-thick red outline):[^outline-syntax]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not critical IMO

@@ -2145,7 +2151,7 @@ The `alert` role addresses this need. A screen reader
will immediately[^alert-css] read an element with that role, no matter
where in the document the user currently is. Note that there aren't
any HTML elements whose default role is `alert`, so this requires
setting the `role` attribute.
the page author to explicitly set the `role` attribute.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not critical IMO

.
<br>
.
<br> . <br> . <br> . <br> . <br> . <br> .
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not critical IMO, but OTOH it does make the proof look a lot better...

@chrishtr chrishtr merged commit e6831ab into main Aug 15, 2024
1 check passed
@chrishtr chrishtr deleted the pp-access branch August 15, 2024 20:27
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.

2 participants