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: add @role for elements #190

Draft
wants to merge 46 commits into
base: master
Choose a base branch
from
Draft

Conversation

AcesAndEights
Copy link

@AcesAndEights AcesAndEights commented Jul 31, 2019

Добавил role для

  • Block
  • BlockAccordion
  • BlockContent
  • Input
  • Button (brand / simple)
  • CheckboxField
  • Checkbox
  • HeaderMenu
  • HeaderMenuItem
  • BlockLink
  • FileLink
  • Link
  • NavLink
  • LogoBlockLink
  • List
  • List elements
  • Simple Modal
  • Exit simple modal
  • Notice
  • Pagination
  • PaginationLink
  • PasswordField
  • RadioField
  • RadioFieldItem
  • SwitchField
  • Switch

@AcesAndEights
Copy link
Author

@dimonka83 @snoosmoomrik

@AcesAndEights AcesAndEights changed the title [NF] Добавил @role к элементам p.1 🚧 feat: add @role for elements Aug 1, 2019
@AcesAndEights AcesAndEights changed the title 🚧 feat: add @role for elements feat: add @role for elements Aug 6, 2019
@dimonka83 dimonka83 changed the title feat: add @role for elements 🚧 feat: add @role for elements Aug 12, 2019
packages/core/src/input/BasicInput.tsx Outdated Show resolved Hide resolved
packages/core/src/icon/Icon.tsx Outdated Show resolved Hide resolved
packages/desktop/src/breadcrumbs/Breadcrumbs.tsx Outdated Show resolved Hide resolved
packages/desktop/src/checkbox-field/CheckboxField.tsx Outdated Show resolved Hide resolved
packages/desktop/src/header-menu/HeaderMenu.tsx Outdated Show resolved Hide resolved
packages/desktop/src/text-field/TextField.tsx Outdated Show resolved Hide resolved
packages/desktop/src/button/Button.tsx Outdated Show resolved Hide resolved
packages/mobile/src/link/FileLink.tsx Show resolved Hide resolved
packages/mobile/src/link/FileLink.tsx Show resolved Hide resolved
packages/mobile/src/link/Link.tsx Outdated Show resolved Hide resolved
@@ -13,6 +13,8 @@ export interface BlockProps {

export const Block: FC<BlockProps> = (props) => (
<Card
role="section"
aria-label="block"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Атрибут aria-label создаёт текстовую метку текущего элемента в случае отсутствия видимого текста описания элемента. зачем тут просто block всегда?

@@ -23,7 +23,7 @@ export const Grid: FC<GridProps> = ({gutter = 20, columns = 12, layout = columns
const rowBlocksCount: number = columns / layoutSum * layoutLength

return (
<Flex wrap="wrap">
<Flex wrap="wrap" role="section" aria-label="grid">
Copy link
Collaborator

Choose a reason for hiding this comment

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

есть роль grid и опять label один для всех и не понятно зачем он тут

@@ -200,7 +200,7 @@ export const Icon: FC<IconProps> = ({
size = 6,
color = '#000',
}) => (
<Svg width={size} height={size} viewBox="0 0 24 24" focusable="false">
<Svg width={size} height={size} viewBox="0 0 24 24" focusable="false" role="img" alt={name}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

как я понял тут aria-label нужен, а не alt

@@ -54,6 +54,7 @@ export const BasicInput: FunctionComponent<BasicInputProps> = (props) => {
onBlur: props.onBlur,
onKeyDown: props.onKeyDown,
onKeyUp: props.onKeyUp,
role: 'textbox',
Copy link
Collaborator

Choose a reason for hiding this comment

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

перед хендлерами

@@ -7,6 +7,7 @@ export interface LinkControlProps {
onFocus?: () => void
onBlur?: () => void
href?: string
title?: string
Copy link
Collaborator

Choose a reason for hiding this comment

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

зачем?

@@ -9,6 +9,7 @@ interface SvgProps {
transition?: string
transform?: string
transformOrigin?: string
alt?: string
Copy link
Collaborator

Choose a reason for hiding this comment

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

как выше писал, возможно aria-label, а не alt

@@ -27,6 +27,7 @@ const BlockAccordionIndent: {

export const BlockAccordion: FunctionComponent<BlockAccordionProps<BlockAccordionItemModel>> = ({items, indent = 'm', tabIndex = 0, opened, onChange}) => (
<AccordionControl<BlockAccordionItemModel>

Copy link
Collaborator

Choose a reason for hiding this comment

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

лишняя строчка

@@ -45,6 +45,7 @@ const TextColor: { [color in NonNullable<TextProps['color']>]: string } = {
export const Text: FunctionComponent<TextProps> = ({display, compact, size, bold, color, decoration, transform, transition, align, clamp, children}) => (
<Typo
as="span"
role="textbox"
Copy link
Collaborator

Choose a reason for hiding this comment

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

зачем у текста textbox?

@dimonka83 dimonka83 changed the title 🚧 feat: add @role for elements feat: add @role for elements Aug 3, 2022
@dimonka83 dimonka83 marked this pull request as draft August 3, 2022 04:40
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