Skip to content

Conversation

@mofojed
Copy link
Member

@mofojed mofojed commented Jul 28, 2021

Write out an error message when the API can't load.

Since we depend on the API for our initialization (eg. ChartBuilder depends on dh.plot.SeriesPlotStyle.* to initialize it's static types), we can't count on our app fully loading and/or executing our index.jsx entrypoint. It may be possible to style this to look better, but it may be a pain. I think this way gets the message across instead of just giving the user a blank screen.

@mofojed mofojed requested a review from dsmmcken July 28, 2021 16:06
@mofojed mofojed changed the title WIP trying out some things to show when the API isn't found Show an error message if the API is not found Jul 28, 2021
@mofojed
Copy link
Member Author

mofojed commented Jul 28, 2021

Nevermind this only works in dev, not in a production build :(

This now handles both cases where the URL for the API isn't found at all (no script loaded), or if an incorrect file is loaded (eg. a 404 file) and script doesn't initialize. Also a bit cleaner.
Copy link
Contributor

@dsmmcken dsmmcken left a comment

Choose a reason for hiding this comment

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

Unrelated, but why not fix the advanced filter creator error, there's an explicit proptype we have in the shim.

import { dh, PropTypes as DhPropTypes } from '@deephaven/jsapi-shim';
...
filters: PropTypes.arrayOf(DhPropTypes.FilterCondition).isRequired,

- Fix up AdvancedFilterCreatorSelectValueList PropType
- Fix up styling of error message
@mofojed mofojed requested a review from dsmmcken July 28, 2021 18:38
@mofojed mofojed self-assigned this Jul 30, 2021
@mofojed
Copy link
Member Author

mofojed commented Jul 30, 2021

Fixes #64

@mofojed mofojed added the enhancement New feature or request label Jul 30, 2021
@mofojed mofojed added this to the July 2021 milestone Jul 30, 2021
@mofojed mofojed linked an issue Jul 30, 2021 that may be closed by this pull request
@mofojed mofojed merged commit 2a9239f into deephaven:main Jul 30, 2021
@mofojed mofojed deleted the 64-api-not-found branch July 30, 2021 14:10
@mofojed mofojed mentioned this pull request Jul 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error message when unable to find API is non-intuitive

2 participants