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

[Embeddables Rebuild] Migrate Image #174962

Closed
Tracked by #167429
ThomThomson opened this issue Jan 16, 2024 · 2 comments · Fixed by #178544
Closed
Tracked by #167429

[Embeddables Rebuild] Migrate Image #174962

ThomThomson opened this issue Jan 16, 2024 · 2 comments · Fixed by #178544
Assignees
Labels
Feature:Embeddables Relating to the Embeddable system impact:critical This issue should be addressed immediately due to a critical level of impact on the product. loe:small Small Level of Effort project:embeddableRebuild Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas

Comments

@ThomThomson
Copy link
Contributor

ThomThomson commented Jan 16, 2024

As part of the Embeddable refactor, we will need to transition the Image Embeddable to the new framework.

@botelastic botelastic bot added the needs-team Issues missing a team label label Jan 16, 2024
@ThomThomson ThomThomson added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas impact:critical This issue should be addressed immediately due to a critical level of impact on the product. Feature:Embeddables Relating to the Embeddable system labels Jan 16, 2024
@botelastic botelastic bot removed the needs-team Issues missing a team label label Jan 16, 2024
@ThomThomson ThomThomson added the loe:small Small Level of Effort label Jan 16, 2024
@Heenawter Heenawter self-assigned this Mar 8, 2024
@Heenawter
Copy link
Contributor

Blocked by #178742

@Heenawter
Copy link
Contributor

Blocked by #179548

@Heenawter Heenawter removed the blocked label Apr 3, 2024
Heenawter added a commit that referenced this issue Apr 8, 2024
…le framework (#178544)

Closes #174962
Closes #165848
Closes #179521

## Summary

This PR converts the Image embeddable to the new React embeddable
framework. There should not be **any** changes in user-facing behaviour
(unless they were made intentionally, such as the small change described
[here](#178544 (comment)))
- therefore, testing of this PR should be focused on ensuring that no
behaviour is changed and/or broken with this refactor.

Since I was already doing a major overhaul, I took the opportunity to
clean up some of the image embeddable code, such as the small change
described
[here](#178544 (comment)).
Some of my changes are heavily influenced by the Presentation team style
(such as my changes to the file organization) so, if there are any
disagreements, I am 100% open to make changes - after all, this code
does not belong to us and we are not responsible for maintenance. Since
this is the first embeddable to be officially refactored (🎉), I expect
there to be lots of questions + feedback and that is okay!

### Small Style Changes
In order to close #165848, I did
two things:
1. I fixed the contrast of the `optionsMenuButton` as described in
#178544 (comment)
2. I ensured that the `PresentationPanel` enforces rounded corners in
view mode while keeping appearances consistent in edit mode (i.e. the
upper corners remain square so that it looks consistent with the title
bar):
    
    | | Before | After |
    |-----|--------|--------|
| **View mode** |
![image](https://github.com/elastic/kibana/assets/8698078/5ecda06f-a47d-41c5-9370-cf51af1489e4)
|
![image](https://github.com/elastic/kibana/assets/8698078/b3503016-63e2-4f70-9600-aa12fd650a67)
|
| **Edit mode** |
![image](https://github.com/elastic/kibana/assets/8698078/bf014f11-8e77-4814-8df3-b1d4cd780bf4)
|
![image](https://github.com/elastic/kibana/assets/8698078/3d8f4606-3d61-48b7-a2d0-f8e4787b8315)
|

### Checklist

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
([FTR](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5559))

![image](https://github.com/elastic/kibana/assets/8698078/2041b786-adfd-4f6e-b885-bc348a4a9e20)
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Embeddables Relating to the Embeddable system impact:critical This issue should be addressed immediately due to a critical level of impact on the product. loe:small Small Level of Effort project:embeddableRebuild Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants