Help tab: add groups #15038

[Andreas-Kainz]

Change-Id: Ic026cc0f4c0d0e47d37dd534f0c5467ecce45c12

• Resolves: Help tab: add groups #15038
• Target version: main

Summary

Use 'type': 'overflowgroup' for each group in the home tab.

[Andreas-Kainz]

As follow up, I will sync menubar and maybe mobil (have to check how it look there) to have everywhere the same command groups and arrangement.

[pedropintosilva]

Thanks @Darshan-upadhyay1110 for the review

[pedropintosilva]

@Andreas-Kainz this seems already approved, it seems it didn't got merged because of conflitcts

[pedropintosilva]

let me double check again this, and attempt to solve the conflicts

[Andreas-Kainz]

I have to update the PR. There was the Diskussion Open, If the Keyboard shortcuts are in accessibility group or a separate group.

[pedropintosilva]

I have to update the PR. There was the Diskussion Open, If the Keyboard shortcuts are in accessibility group or a separate group.

#15065 (comment)

[pedropintosilva]

I have rebased and solved the conflitcts

[pedropintosilva]

I have also tested and if the questions was were to place it? In Help or Accessibility top hierarchy level? I would say Help. But that is not the question as anyhow the button is within the top level Help (tab). So, either way I think we are ok.

so, I think after it passes we can merge.

[Andreas-Kainz]

thank's for the reviews. how can it go to main? do I have to rebuild?

[Darshan-upadhyay1110]

thank's for the reviews. how can it go to main? do I have to rebuild?

@Andreas-Kainz it looks good, could you please rebase it once more? I think the CI failure might just be a false positive.

[Andreas-Kainz]

can the PR go to main?

[pedropintosilva]

Sorry for taking too long to merge this but did you @Darshan-upadhyay1110 or @Andreas-Kainz run the failing test?

Test failed: integration_tests/desktop/calc/a11y_notebookbar_spec.js / Accessibility Calc Notebookbar Tests / All non-context notebookbar tabs
                    
                    Found A11y errors:
                    A11yValidatorException: Tab 'Help-tab-label': shortcut 'S' on 'keyboard-shortcuts' is a prefix of 'SA' on 'server-audit', making 'server-audit' unreachable.
                    

It seems there is a problem with the access keys (you can test it by pressing alt or in the case of FF shift + Alt) particularly with accessing server audit button.

[Andreas-Kainz]

How to run the a11 test?

I checked now all the getHelpTab commands, but I think there was no changes done. What is here failing?

I'm not a big fan of the a11y combination in the codebase.

[pedropintosilva]

According to the error above^ it is unreachable via access keys .

To answer your question: https://github.com/CollaboraOnline/online/blob/main/cypress_test%2FREADME

But I see that the access key on that button remained unchanged, maybe something elese n grouping triggered this

[Darshan-upadhyay1110]

Sorry for taking too long to merge this but did you @Darshan-upadhyay1110 or @Andreas-Kainz run the failing test?

Test failed: integration_tests/desktop/calc/a11y_notebookbar_spec.js / Accessibility Calc Notebookbar Tests / All non-context notebookbar tabs
                    
                    Found A11y errors:
                    A11yValidatorException: Tab 'Help-tab-label': shortcut 'S' on 'keyboard-shortcuts' is a prefix of 'SA' on 'server-audit', making 'server-audit' unreachable.
                    

It seems there is a problem with the access keys (you can test it by pressing alt or in the case of FF shift + Alt) particularly with accessing server audit button.

Ahhhh Ci is really pain now days i am testing the case now :( Thanks pedro for the comment

[Darshan-upadhyay1110]

Now i see why cypress failed for a11y

As shown in the image, we can’t use just S as a keyboard shortcut because there are already several two-key combinations that start with S. In such cases, the two-key shortcuts would never be triggered.

Please change this key combination to it's original one.

[Darshan-upadhyay1110]

And @Andreas-Kainz, could you Pretty please test the changes after creating the PR? These are quite large changes, and small issues can easily be missed. For example, in your recent PR there were some minor issues like IDs getting different combinations than the original, and a few items having different commands than intended.

[Andreas-Kainz]

And @Andreas-Kainz, could you Pretty please test the changes after creating the PR? These are quite large changes, and small issues can easily be missed. For example, in your recent PR there were some minor issues like IDs getting different combinations than the original, and a few items having different commands than intended.

I'm sorry that I get that much troubles. Maybe we can talk at cool days to improve it.

[Andreas-Kainz]

see https://gerrit.collaboraoffice.com/c/online/+/1791