Remove waitForSelector for some dashboards page #10
Conversation
Signed-off-by: Rupal Mahajan <[email protected]>
Signed-off-by: Rupal Mahajan <[email protected]>
Signed-off-by: Rupal Mahajan <[email protected]>
|
Attaching few sample reports generated with this change opensearch-report-2023-01-20T21:48:50.296Z.pdf |
Signed-off-by: Rupal Mahajan <[email protected]>
src/download-helpers.js
Outdated
| const reportSource = getReportSourceFromURL(url); | ||
|
|
||
| // if its an OpenSearch report, remove extra elements. | ||
| if (reportSource !== 'Other' && reportSource !== 'Saved search') { |
There was a problem hiding this comment.
I remember removing extra buttons was implemented to reduce unnecessary information in the report, is it ok to delete this?
If other page url (e.g. alerting) is used, reportSource would be Other so this logic and waitForSelector should be skipped. This should not make a difference?
There was a problem hiding this comment.
For alerinng it will work, but I tried anomaly detection, url: <dashboard_endpoint>/_dashboards/app/anomaly-detection-dashboards#/
Since url had dashboards#' cli got stuck waiting for #dashboardViewport
There was a problem hiding this comment.
I think we can keep this if we have some identifiers which can't be part of other opensearch url's. I also compared the pdf report with and without this change. There's not much visible differences so I think it's ok to remove this for now. Do you have any other suggestion?
There was a problem hiding this comment.
For alerinng it will work, but I tried anomaly detection, url:
<dashboard_endpoint>/_dashboards/app/anomaly-detection-dashboards#/Since url haddashboards#'cli got stuck waiting for#dashboardViewport
You can make it matching more specific, maybe /app/dashboards#. It's ok if not much difference, did you try visualize though? I remember it removes the vis editor
There was a problem hiding this comment.
Earlier I only tried dashboard, just tried visualize. I didn't notice much difference.
With main branch: opensearch-report-2023-01-20T22:41:33.335Z-main.pdf
With this change: opensearch-report-2023-01-20T22:39:37.417Z-fix.pdf
There was a problem hiding this comment.
change the constant to
export const URL_SOURCE = {
DASHBOARDS: "/app/dashboards#",
VISUALIZE: "/app/visualize#",
DISCOVER: "/app/discover#",There was a problem hiding this comment.
Added this change and removed waitForSelector.
Signed-off-by: Rupal Mahajan <[email protected]> This reverts commit a18a67e.
Signed-off-by: Rupal Mahajan <[email protected]>
Signed-off-by: Rupal Mahajan <[email protected]> This reverts commit c1834f6.
rupal-bq
changed the title
Description
Issues Resolved
List any issues this PR will resolve, e.g. Closes [...].
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.