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

[Tabs] M3-4119: Refactor Tabs - Lish #6467

Merged
merged 9 commits into from Jun 22, 2020

Conversation

tiffwong
Copy link
Contributor

@tiffwong tiffwong commented May 22, 2020

Description

Refactor Lish tabs to use Reach UI instead.

Type of Change

  • Non breaking change ('update', 'change')

Note to Reviewers

The tabs can now be navigated with arrow keys but input only shows up in Glish. Tabbing into Weblish and Glish kinda work in terminal so there might be some kind of conflict being the Reach tabs and the usage of a terminal and websocket. I'll open a new ticket for this 💁‍♀️

The tabs and terminals are now keyboard accessible, there is an extra tab required to get into the LISH console so I'll look into that but otherwise it should be good

@tiffwong tiffwong self-assigned this May 22, 2020
@tiffwong tiffwong changed the base branch from develop to feature/tab-refactor May 22, 2020 18:58
Copy link
Contributor

@Jskobos Jskobos left a comment

Choose a reason for hiding this comment

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

I agree Lish is a special case. One idea would be to add event listeners for cmd+alt+[], so switching from Lish to Glish is like switching browser tabs (which you can't do in the Lish window). Or maybe find a way to have Tab be the way to switch tabs in this one case.

Copy link
Contributor

@WilkinsKa1 WilkinsKa1 left a comment

Choose a reason for hiding this comment

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

Looking good! Couple of comments, one being a potential solution to the Glish issue.

))}
<TabLinkList lish tabs={this.tabs} />
<TabPanels>
<TabPanel data-qa-tab="Weblish" {...tabA11yProps('Weblish')}>
Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove the tabA11yProps here and on line 216- these are now being applied automatically by Reach instead!

indicatorColor="primary"
textColor="primary"
scrollButtons="off"
defaultIndex={this.tabs.findIndex(tab => this.matches(tab.routeName))}
Copy link
Contributor

Choose a reason for hiding this comment

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

In re to the issue of keypresses being read by Glish when navigating between the consoles, Reach actually has this prop https://reacttraining.com/reach-ui/tabs/#tabs-keyboardactivation where you can control the way the tabPanel is activated. I think this could potentially prevent the inputted text from entering the Glish console during navigation as the panel won't ever be activated unless the user presses enter/space.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the prop but it doesn't seem to do anything 🤔

Copy link
Contributor

@WilkinsKa1 WilkinsKa1 left a comment

Choose a reason for hiding this comment

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

Nice work on this! I did push up an edit to remove redundant a11y attributes (Reach handles those for us now instead)

Copy link
Contributor

@johnwcallahan johnwcallahan left a comment

Choose a reason for hiding this comment

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

You'll need to use SafeTabPanel (which you'll need a rebase for). Currently both Lish and Glish are rendered and listening for keystrokes, so when you type in one, you see the keystrokes in both.

@@ -60,6 +85,7 @@ export interface Tab {
interface Props {
tabs: Tab[];
[index: string]: any;
lish?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of having TabLinkList handle the CSS for Lish, it'd be more scalable if it could accept a CSS classname so it's controllable by the parent. An example can be found here: https://github.com/linode/manager/blob/develop/packages/manager/src/components/PromotionalOfferCard/PromotionalOfferCard.tsx#L91

// TODO: Change type 'any' to a more appropriate type
const ref = React.useRef<any>(null);

React.useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly, if this Ref is only here for Lish (for now), it should be controlled by the parent and passed down as a prop.

@acourdavault acourdavault added the Accessibility Contains accessibility improvements or changes label Jun 3, 2020
@tiffwong tiffwong merged commit 7916fde into feature/tab-refactor Jun 22, 2020
@tiffwong tiffwong deleted the M3-4119-refactor-tabs-lish branch July 1, 2020 19:03
WilkinsKa1 pushed a commit that referenced this pull request Aug 5, 2020
* Refactor to Reach UI

* Add keyboardActivation prop

* Working on focusing on tabs

* Add ref to tabs

* removing redundant a11y attributes

* Switch to SafeTabPanel

* Move styles to Lish and get rid of ref

* Remove TabPanel import

Co-authored-by: Kayla Wilkins <kwilkins@linode.com>
WilkinsKa1 pushed a commit that referenced this pull request Aug 7, 2020
* Refactor Tabbed Panel Component & Linode Details Navigation (#6331)

* adding in arrows + functionality, beginnings of how to dynamically hide them

* converting to a FC, adding in some state mgt for mobile buttons

* remove mui mobile buttons

* beginning to convert a tab nav with bare bones reach

* using as for tabpanels so path routing works

* using as for tabpanel prop and applying path

* adding defaultIndex, removing old aria cruff from settings

* fixing settings issue

* fixing linting issues

* loading css in app instead of each individual reach component

* adding documentation for these tab patterns, referencing in contributing doc

* Convert test to RTL and resolve key warnings

* Tweak tab keys

* Addressing PR feedback part 1

* fixing focus outline cutoff issue

Co-authored-by: Jared Kobos <jaredkobos@gmail.com>

* Tab Refactor: Account Landing, OBJ Landing, FW Details (#6375)

* updating account landing, FW detail and OBJ landing to new linked tab pattern

* fixing index for key issue

* Tab Refactor: LKE Details, Support Tickets Landing, Linode Clone Landing  (#6378)

* updating to use reach tab pattern for LKE detail, tickets landing, and clone landing

* remove handleTabChange

* Refactor Tabbed Panel Component & Linode Details Navigation (#6331)

* adding in arrows + functionality, beginnings of how to dynamically hide them

* converting to a FC, adding in some state mgt for mobile buttons

* remove mui mobile buttons

* beginning to convert a tab nav with bare bones reach

* using as for tabpanels so path routing works

* using as for tabpanel prop and applying path

* adding defaultIndex, removing old aria cruff from settings

* fixing settings issue

* fixing linting issues

* loading css in app instead of each individual reach component

* adding documentation for these tab patterns, referencing in contributing doc

* Convert test to RTL and resolve key warnings

* Tweak tab keys

* Addressing PR feedback part 1

* fixing focus outline cutoff issue

Co-authored-by: Jared Kobos <jaredkobos@gmail.com>

* M3-4128 Refactor User details, NodeBalancer details, Profile, Longview landing and details (#6423)

* changing tab patterns for user details, Nodebalancer details, and profile + cleanup

* attempting to fix oauth and NB issues

* removing dupe props issue

* changing tab patterns for user details, Nodebalancer details, and profile + cleanup

* attempting to fix oauth and NB issues

* removing stray again after branch cleanup debacle

* adding new SafeTabPanel abstraction to load content only on active tab selection instead of all

* longview POC with safe tab panel

* adding longview tabs with SafeTabPanel to fix graphs loading issue

* checking for possible indexes based on existence of apps for a LV client

* Add comment to SafeTabPanel

Co-authored-by: John Callahan <johnwcallahan@gmail.com>

* Managed Tabs Refactor (#6462)

* refactoring managed tabs to use reach

* fixing alignment of managed tabs

* update to using SafeTabPanel

* Tab Panel Refinements (#6477)

* removing old a11y attributes from tabPanels that have been refactored so far, fixing alignment on misaligned panels

* removing focus if header is rendered as an h2

* Refactor Tabbed Panel Component & Linode Details Navigation (#6331)

* adding in arrows + functionality, beginnings of how to dynamically hide them

* converting to a FC, adding in some state mgt for mobile buttons

* remove mui mobile buttons

* beginning to convert a tab nav with bare bones reach

* using as for tabpanels so path routing works

* using as for tabpanel prop and applying path

* adding defaultIndex, removing old aria cruff from settings

* fixing settings issue

* fixing linting issues

* loading css in app instead of each individual reach component

* adding documentation for these tab patterns, referencing in contributing doc

* Convert test to RTL and resolve key warnings

* Tweak tab keys

* Addressing PR feedback part 1

* fixing focus outline cutoff issue

Co-authored-by: Jared Kobos <jaredkobos@gmail.com>

* Tab Refactor: Account Landing, OBJ Landing, FW Details (#6375)

* updating account landing, FW detail and OBJ landing to new linked tab pattern

* fixing index for key issue

* Tab Refactor: LKE Details, Support Tickets Landing, Linode Clone Landing  (#6378)

* updating to use reach tab pattern for LKE detail, tickets landing, and clone landing

* remove handleTabChange

* M3-4128 Refactor User details, NodeBalancer details, Profile, Longview landing and details (#6423)

* changing tab patterns for user details, Nodebalancer details, and profile + cleanup

* attempting to fix oauth and NB issues

* removing dupe props issue

* changing tab patterns for user details, Nodebalancer details, and profile + cleanup

* attempting to fix oauth and NB issues

* removing stray again after branch cleanup debacle

* adding new SafeTabPanel abstraction to load content only on active tab selection instead of all

* longview POC with safe tab panel

* adding longview tabs with SafeTabPanel to fix graphs loading issue

* checking for possible indexes based on existence of apps for a LV client

* Add comment to SafeTabPanel

Co-authored-by: John Callahan <johnwcallahan@gmail.com>

* [Tabs] M3-4119: Refactor Tabs - Lish (#6467)

* Refactor to Reach UI

* Add keyboardActivation prop

* Working on focusing on tabs

* Add ref to tabs

* removing redundant a11y attributes

* Switch to SafeTabPanel

* Move styles to Lish and get rid of ref

* Remove TabPanel import

Co-authored-by: Kayla Wilkins <kwilkins@linode.com>

* Refactor Tabbed Panel Component & Linode Details Navigation (#6331)

* adding in arrows + functionality, beginnings of how to dynamically hide them

* converting to a FC, adding in some state mgt for mobile buttons

* remove mui mobile buttons

* beginning to convert a tab nav with bare bones reach

* using as for tabpanels so path routing works

* using as for tabpanel prop and applying path

* adding defaultIndex, removing old aria cruff from settings

* fixing settings issue

* fixing linting issues

* loading css in app instead of each individual reach component

* adding documentation for these tab patterns, referencing in contributing doc

* Convert test to RTL and resolve key warnings

* Tweak tab keys

* Addressing PR feedback part 1

* fixing focus outline cutoff issue

Co-authored-by: Jared Kobos <jaredkobos@gmail.com>

* Tab Refactor: Account Landing, OBJ Landing, FW Details (#6375)

* updating account landing, FW detail and OBJ landing to new linked tab pattern

* fixing index for key issue

* Tab Refactor: LKE Details, Support Tickets Landing, Linode Clone Landing  (#6378)

* updating to use reach tab pattern for LKE detail, tickets landing, and clone landing

* remove handleTabChange

* M3-4128 Refactor User details, NodeBalancer details, Profile, Longview landing and details (#6423)

* changing tab patterns for user details, Nodebalancer details, and profile + cleanup

* attempting to fix oauth and NB issues

* removing dupe props issue

* changing tab patterns for user details, Nodebalancer details, and profile + cleanup

* attempting to fix oauth and NB issues

* removing stray again after branch cleanup debacle

* adding new SafeTabPanel abstraction to load content only on active tab selection instead of all

* longview POC with safe tab panel

* adding longview tabs with SafeTabPanel to fix graphs loading issue

* checking for possible indexes based on existence of apps for a LV client

* Add comment to SafeTabPanel

Co-authored-by: John Callahan <johnwcallahan@gmail.com>

* M3-4128 Refactor User details, NodeBalancer details, Profile, Longview landing and details (#6423)

* changing tab patterns for user details, Nodebalancer details, and profile + cleanup

* attempting to fix oauth and NB issues

* removing dupe props issue

* changing tab patterns for user details, Nodebalancer details, and profile + cleanup

* attempting to fix oauth and NB issues

* removing stray again after branch cleanup debacle

* adding new SafeTabPanel abstraction to load content only on active tab selection instead of all

* longview POC with safe tab panel

* adding longview tabs with SafeTabPanel to fix graphs loading issue

* checking for possible indexes based on existence of apps for a LV client

* Add comment to SafeTabPanel

Co-authored-by: John Callahan <johnwcallahan@gmail.com>

* Managed Tabs Refactor (#6462)

* refactoring managed tabs to use reach

* fixing alignment of managed tabs

* update to using SafeTabPanel

* Tab Panel Refinements (#6477)

* removing old a11y attributes from tabPanels that have been refactored so far, fixing alignment on misaligned panels

* removing focus if header is rendered as an h2

* fixing tabs

* Refactor Tabbed Panel Component & Linode Details Navigation (#6331)

* adding in arrows + functionality, beginnings of how to dynamically hide them

* converting to a FC, adding in some state mgt for mobile buttons

* remove mui mobile buttons

* beginning to convert a tab nav with bare bones reach

* using as for tabpanels so path routing works

* using as for tabpanel prop and applying path

* adding defaultIndex, removing old aria cruff from settings

* fixing settings issue

* fixing linting issues

* loading css in app instead of each individual reach component

* adding documentation for these tab patterns, referencing in contributing doc

* Convert test to RTL and resolve key warnings

* Tweak tab keys

* Addressing PR feedback part 1

* fixing focus outline cutoff issue

Co-authored-by: Jared Kobos <jaredkobos@gmail.com>

* Tab Refactor: Account Landing, OBJ Landing, FW Details (#6375)

* updating account landing, FW detail and OBJ landing to new linked tab pattern

* fixing index for key issue

* Tab Refactor: LKE Details, Support Tickets Landing, Linode Clone Landing  (#6378)

* updating to use reach tab pattern for LKE detail, tickets landing, and clone landing

* remove handleTabChange

* M3-4128 Refactor User details, NodeBalancer details, Profile, Longview landing and details (#6423)

* changing tab patterns for user details, Nodebalancer details, and profile + cleanup

* attempting to fix oauth and NB issues

* removing dupe props issue

* changing tab patterns for user details, Nodebalancer details, and profile + cleanup

* attempting to fix oauth and NB issues

* removing stray again after branch cleanup debacle

* adding new SafeTabPanel abstraction to load content only on active tab selection instead of all

* longview POC with safe tab panel

* adding longview tabs with SafeTabPanel to fix graphs loading issue

* checking for possible indexes based on existence of apps for a LV client

* Add comment to SafeTabPanel

Co-authored-by: John Callahan <johnwcallahan@gmail.com>

* M3-4128 Refactor User details, NodeBalancer details, Profile, Longview landing and details (#6423)

* changing tab patterns for user details, Nodebalancer details, and profile + cleanup

* attempting to fix oauth and NB issues

* removing dupe props issue

* changing tab patterns for user details, Nodebalancer details, and profile + cleanup

* attempting to fix oauth and NB issues

* removing stray again after branch cleanup debacle

* adding new SafeTabPanel abstraction to load content only on active tab selection instead of all

* longview POC with safe tab panel

* adding longview tabs with SafeTabPanel to fix graphs loading issue

* checking for possible indexes based on existence of apps for a LV client

* Add comment to SafeTabPanel

Co-authored-by: John Callahan <johnwcallahan@gmail.com>

* Managed Tabs Refactor (#6462)

* refactoring managed tabs to use reach

* fixing alignment of managed tabs

* update to using SafeTabPanel

* Tab Panel Refinements (#6477)

* removing old a11y attributes from tabPanels that have been refactored so far, fixing alignment on misaligned panels

* removing focus if header is rendered as an h2

* Refactor Tabbed Panel Component & Linode Details Navigation (#6331)

* adding in arrows + functionality, beginnings of how to dynamically hide them

* converting to a FC, adding in some state mgt for mobile buttons

* remove mui mobile buttons

* beginning to convert a tab nav with bare bones reach

* using as for tabpanels so path routing works

* using as for tabpanel prop and applying path

* adding defaultIndex, removing old aria cruff from settings

* fixing settings issue

* fixing linting issues

* loading css in app instead of each individual reach component

* adding documentation for these tab patterns, referencing in contributing doc

* Convert test to RTL and resolve key warnings

* Tweak tab keys

* Addressing PR feedback part 1

* fixing focus outline cutoff issue

Co-authored-by: Jared Kobos <jaredkobos@gmail.com>

* Refactor Tabbed Panel Component & Linode Details Navigation (#6331)

* adding in arrows + functionality, beginnings of how to dynamically hide them

* converting to a FC, adding in some state mgt for mobile buttons

* remove mui mobile buttons

* beginning to convert a tab nav with bare bones reach

* using as for tabpanels so path routing works

* using as for tabpanel prop and applying path

* adding defaultIndex, removing old aria cruff from settings

* fixing settings issue

* fixing linting issues

* loading css in app instead of each individual reach component

* adding documentation for these tab patterns, referencing in contributing doc

* Convert test to RTL and resolve key warnings

* Tweak tab keys

* Addressing PR feedback part 1

* fixing focus outline cutoff issue

Co-authored-by: Jared Kobos <jaredkobos@gmail.com>

* [Tabs] M3-4119: Refactor Tabs - Lish (#6467)

* Refactor to Reach UI

* Add keyboardActivation prop

* Working on focusing on tabs

* Add ref to tabs

* removing redundant a11y attributes

* Switch to SafeTabPanel

* Move styles to Lish and get rid of ref

* Remove TabPanel import

Co-authored-by: Kayla Wilkins <kwilkins@linode.com>

* fixing tabs

* Linode Create Tab Refactor (#6661)

* In progress

* Extract add-on panel

* Extract label/tags and password

* Clean up code

* Moving checkout bar over in-progress

* Moving on to Marketplace

* temp

* temp

* commenting out repeating code, putting one checkoout bar + functions in linode create

* Stackscripts tab contains account and community

* Cleanup

* Working on ImageSelect

* Fix ImageSelect issue

* Remove console

* Cleanup

* More cleanup

* Convert FromAppsContent to FC

* Start adding types back

* Fix merge conflict

* adjustments to type interfaces for create refactor to fix prop errors

* fixing tab routing issue and updating marketplace to one click for marketing match

* adjustments to linking and conditional wrapping of components for clone and backup

* adjustment to be able to create from clone

* Clean up fromApps and fromStackscript

* Clean up props and in-progress converting classes to FC

* Fix routing issue

* Minor fixes

* adjustemnt to type definiton, props cleanup for LinodeCreate

* adding it all back

* fixing props issue

* Addressing PR feeddback part 1

* adding missing error notice

* Adding disabled notice top level in linode create

* fixing broken tests, removing obsolete ones

* reverting test back to RTL

* Adding general error to create, adjusting individual error handling for FromContent

Co-authored-by: Tiffany Wong <wong.tyan@gmail.com>

* removing obsolete subtabs file

* fixing build bc of sentry version

* updating CMR tab components

Co-authored-by: Jared Kobos <jaredkobos@gmail.com>
Co-authored-by: John Callahan <johnwcallahan@gmail.com>
Co-authored-by: Tiffany Wong <7692354+tiffwong@users.noreply.github.com>
Co-authored-by: Tiffany Wong <wong.tyan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accessibility Contains accessibility improvements or changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants