[Files] Move <Image /> component to @kbn/shared-ux package#145995
[Files] Move <Image /> component to @kbn/shared-ux package#145995jloleysens merged 23 commits intoelastic:mainfrom
@kbn/shared-ux package#145995Conversation
jloleysens
added
release_note:skip
| canvas.width = width; | ||
| canvas.height = height; | ||
| const ctx = canvas.getContext('2d'); | ||
| if (!ctx) throw new Error('Could not get 2d canvas context!'); |
There was a problem hiding this comment.
we are throwing an error here and then catching it further below and swallowing it. should we log this somewhere? or should this part be outside of the try-catch block so the error can be surfaced?
There was a problem hiding this comment.
Good point!
Generating a blurhash for an image is kind of a nice-to-have. If it fails for whatever reason we definitely do not want to block the user. However, swallowing this error entirely will hide cases where blurhash generation fails (for whatever reason). It'd be great to record failures somehow, perhaps to our telemetry cluster (some kind of event with a message?). Not sure logging to the console would do much good, unless users actually inform us.
Last thought: could we address this on separate PR?
majagrubic
left a comment
majagrubic
left a comment
There was a problem hiding this comment.
Thank you for doing this! Tested in storybook on mac os X, all looks good. Code changes are pretty straightforward. Just one question below around error catching.
* main: Make page titles more consistent for Overview, Alerts, Rules, Rule Detail and Cases pages (elastic#146150) [Files] Delay `<Image/>` blurhash reveal and handle blurhash errors (elastic#146159)
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
ESLint disabled in files
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
majagrubic
left a comment
majagrubic
left a comment
There was a problem hiding this comment.
Changes look good. Tested in storybook in Chrome on Mac OSX
* main: (30 commits) [Cloud Posture] test latest findings table sort (elastic#144668) [api-docs] 2022-11-28 Daily api_docs build (elastic#146359) [api-docs] 2022-11-27 Daily api_docs build (elastic#146353) [api-docs] 2022-11-26 Daily api_docs build (elastic#146350) [DataViews] Fix form validation UX when the same data view name already exists (elastic#146126) [Discover] Prevent agg based visualizations of Discover saved objects with adhoc data views (elastic#145583) [Health Gateway] Update response aggregation (elastic#145761) [api-docs] 2022-11-25 Daily api_docs build (elastic#146341) [Metric threshold rule] Adds new context variable for group by keys (elastic#145654) [Controls] [Portable Dashboards] Add control group renderer example plugin (elastic#146189) Refactor Observability Overview Page (elastic#146182) Send complete test data to xMatters, so it can create an alert (elastic#145431) [Dashboard] [Controls] Allow options list suggestions to be sorted (elastic#144867) Add open API specification for list connector types (elastic#145951) skip flaky suite (elastic#146086) [ML] Removing duplicate tooltip text (elastic#146308) Refactor Rules Page (elastic#146193) [DOCS] Alert limit for cases (elastic#145950) Extend session index fields mapping with a session creation timestamp. (elastic#145997) [Files] Move <Image /> component to `@kbn/shared-ux` package (elastic#145995) ...
4 participants
Summary
@kbn/shared-ux-file-image,@kbn/shared-ux-file-image-types,@kbn/shared-ux-file-image-mocks@kbn/shared-ux-file-utilpackage for the helpers shared across componentsHow to test
See
yarn storbook shared_ux's new section "Files"Additional notes
First step just focussed on moving
Imagecomponent, we still need to moveFilePickerandUploadFile