-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Headings Hiearchy Fixed #2443
base: main
Are you sure you want to change the base?
Headings Hiearchy Fixed #2443
Conversation
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.
Hi @shubhamt619, thank you for your contribution.
I have a couple of questions/clarifications
@@ -41,7 +47,7 @@ export const RightPanelHeader = styled.h3` | |||
${extensionsHook('RightPanelHeader')}; | |||
`; | |||
|
|||
export const UnderlinedHeader = styled.h5` | |||
export const UnderlinedHeader = styled.h4` |
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.
why do we need to change h5->h4?
@@ -36,7 +36,7 @@ export const Operation = observer(({ operation }: OperationProps): JSX.Element = | |||
{options => ( | |||
<Row {...{ [SECTION_ATTR]: operation.operationHash }} id={operation.operationHash}> | |||
<MiddlePanel> | |||
<H2> | |||
<H3> |
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.
Is it necessary to change level of heading in this case?
What/Why/How?
Headings Hierarchy was missing tags.
Added keyboard navigation to Headings
Fixed #2408
Fixed #2405
Reference
Refer to #2408
Refer to #2405
Tests
All test cases passed, because I updated the test cases where needed.
Screenshots (optional)
Check yourself