[Chore] Visualize link fix

[BSFishy]

Description

Change the links in the visualize plugin to use href rather than onClick. This is beneficial, as it allows links to be opened in new tabs, and helps with accessibility, as the elements are a instead of button now.

Issues Resolved

#2197

Check List

• New functionality includes testing.
  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
    • yarn test:ftr
• New functionality has been documented.
• Commits are signed per the DCO using --signoff

[codecov-commenter]

Codecov Report

Merging #2395 (7a29f5c) into main (06abe83) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2395   +/-   ##
=======================================
  Coverage   66.74%   66.75%           
=======================================
  Files        3194     3194           
  Lines       60803    60803           
  Branches     9238     9238           
=======================================
+ Hits        40585    40589    +4     
+ Misses      18009    18007    -2     
+ Partials     2209     2207    -2     

Impacted Files	Coverage Δ
...ared/static/forms/hook_form_lib/hooks/use_field.ts	65.70% <0.00%> (-0.97%)	⬇️
...ic/application/models/sense_editor/sense_editor.ts	64.88% <0.00%> (+0.88%)	⬆️
packages/osd-optimizer/src/node/cache.ts	52.77% <0.00%> (+2.77%)	⬆️
...s/osd-optimizer/src/node/node_auto_tranpilation.ts	87.75% <0.00%> (+4.08%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[ashwin-pc]

This fix does not work with aliased visualizations. e.g. Vis Builder aka Wizard.

I didn't look too closely into why but the steps to repro this are:

1. Enable Wizard: Set the last flag in opensearch_dashboards.yml config to true wizard.enabled:true
2. Create a new visualization using the wizard type and save it
3. Go to the visualization table and try to edit it

You should see

[BSFishy]

Also, something that just came to me that I think might be up for discussion is link prefetching. It's out of the scope of this issue and PR, but I think that it might be useful. I don't think the regular load speed is much of an issue, but if the link is opened in a new tab, the page can take a little bit of time to load which I think prefetching would help with. Should I open a new issue to discuss this further?

cc @ashwin-pc @kavilla

[bandinib-amzn]

This fix does not work with aliased visualizations. e.g. Vis Builder aka Wizard.

I didn't look too closely into why but the steps to repro this are:

1. Enable Wizard: Set the last flag in opensearch_dashboards.yml config to true wizard.enabled:true
2. Create a new visualization using the wizard type and save it
3. Go to the visualization table and try to edit it

You should see

Hi @ashwin-pc
I followed your steps to repro the issue but couldn't reproduce it. I'm not getting Page not found error. I tried both Edit action and Open Link in new tab/new window.

[BSFishy]

This fix does not work with aliased visualizations. e.g. Vis Builder aka Wizard.
I didn't look too closely into why but the steps to repro this are:

1. Enable Wizard: Set the last flag in opensearch_dashboards.yml config to true wizard.enabled:true
2. Create a new visualization using the wizard type and save it
3. Go to the visualization table and try to edit it

You should see

Hi @ashwin-pc I followed your steps to repro the issue but couldn't reproduce it. I'm not getting Page not found error. I tried both Edit action and Open Link in new tab/new window.

Hey @bandinib-amzn
I actually fixed that bug in this commit. You would probably find it again if you pulled from a previous commit, but it should be fixed now.

[ashwin-pc]

Ship it 🚢

[joshuarrrr]

@BSFishy I really like the direction you've gone with this, but unfortunately it's not sufficient to hardcode the # prefix to the editUrl. The reason that works for the wizard plugin is that we define #/ as the aliasPath here: https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/plugins/wizard/public/plugin.ts#L117 . This may cause regressions for aliased visualizations with other aliasPaths.

My suspicion is that we may need to adjust the way editUrl is fetched for aliased visualizations upstream of this component so that it correctly accounts for the aliasPath.

[BSFishy]

I just did a bit of digging, and I think this should work fine. The reason that it works with the wizard plugin is because it defines its own application, causing it to call getUrlForApp. Otherwise, it will follow the same functionality that was done by navigateToUrl. In that function, a call was made to history.push, which is implemented in the history package. In that package, they simply prepend a # to the beginning of the path:

https://github.com/remix-run/history/blob/3e9dab413f4eda8d6bce565388c5ddb7aeff9f7e/packages/history/index.ts#L693

I can still have it get the url upstream of this component, though, like the DashboardListing component does, however in that component, the getViewUrl function is a property, which I think would be a breaking change.

OpenSearch-Dashboards/src/plugins/dashboard/public/application/listing/dashboard_listing.js

Lines 173 to 177 in 48fe60b

	render: (field, record) => (
	<EuiLink
	href={this.props.getViewUrl(record)}
	data-test-subj={`dashboardListingTitleLink-${record.title.split(' ').join('-')}`}
	>

[joshuarrrr]

Ah, thanks for digging into the history implementation, that makes sense.

[joshuarrrr]

Ah, thanks for digging into the history implementation, that makes sense.

[seraphjiang]

Nice, great job. @BSFishy and Thanks @ashwin-pc @bandinib-amzn @joshuarrrr for the review.