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

[Files] Delay <Image/> blurhash reveal and handle blurhash errors #146159

Merged
merged 3 commits into from
Nov 24, 2022

Conversation

Dosant
Copy link
Contributor

@Dosant Dosant commented Nov 23, 2022

Summary

Follow up to #145347
Needed for #81345
Partially fixes #145567

Two things:

  • Handle errors related to blurhash loading. Make sure that the original image is loaded if blurhash is failed to generate src or failed to load
  • Make blurhash appear after the delay. This is needed for better UX when the original image loads fast. Implemented using css animation where first part of the animation is a static state with opacity: 0, and then the image is revealed in the end. If the original image loads faster, then it appears instantaneously without the delay

@Dosant Dosant added release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) feature:Files labels Nov 23, 2022
@Dosant Dosant marked this pull request as ready for review November 23, 2022 14:50
@Dosant Dosant requested a review from a team as a code owner November 23, 2022 14:50
@Dosant Dosant requested a review from jloleysens November 23, 2022 14:50
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] x-pack/test/alerting_api_integration/security_and_spaces/group1/config.ts / alerting api integration security and spaces enabled Alerts - Group 1 alerts bulkDisableRules space_1_all_alerts_none_actions at space1 should handle disable alert request appropriately when consumer is "alerts"
  • [job] [logs] x-pack/test/alerting_api_integration/security_and_spaces/group1/config.ts / alerting api integration security and spaces enabled Alerts - Group 1 alerts bulkDisableRules space_1_all_with_restricted_fixture at space1 should handle bulk disable of one rule appropriately based on id
  • [job] [logs] x-pack/test/alerting_api_integration/security_and_spaces/group1/config.ts / alerting api integration security and spaces enabled Alerts - Group 1 alerts bulkDisableRules space_1_all_with_restricted_fixture at space1 should handle bulk disable of one rule appropriately based on id when consumer is the same as producer

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
files 17.5KB 17.9KB +462.0B
Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
files 2 4 +2
fleet 59 65 +6
osquery 109 115 +6
securitySolution 443 449 +6
total +22

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
files 6 8 +2
fleet 68 74 +6
osquery 110 117 +7
securitySolution 520 526 +6
total +23

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

Very nice enhancements, tested locally and it looks really good! Great job @Dosant !

@Dosant Dosant merged commit f6b46ce into elastic:main Nov 24, 2022
@kibanamachine kibanamachine added v8.7.0 backport:skip This commit does not require backporting labels Nov 24, 2022
jloleysens added a commit to jloleysens/kibana that referenced this pull request Nov 24, 2022
* 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting feature:Files release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v8.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Files] <Image/> blurhash support improvements
4 participants