[Maps] add fit to bounds button#28801
Conversation
thomasneirynck
added
WIP
|
Pinging @elastic/kibana-gis |
💚 Build Succeeded |
💔 Build Failed |
💔 Build Failed |
thomasneirynck
force-pushed
the
maps/add_fittobounds
branch
from
18a4799 to
aef7553
Compare
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
thomasneirynck
force-pushed
the
maps/add_fittobounds
branch
from
3ebee80 to
3f9a0d3
Compare
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
|
Here is a PR4U: thomasneirynck#3 Also, I still consistently get the base layer name not showing up on new maps and the Longitude decimal being way too long. As well as the toggle visibility button doesn't seem to work |
💚 Build Succeeded |
|
Zoom visibility tooltip overlaps action panel. Maybe the tooltip needs to go up or something
|
| import _ from 'lodash'; | ||
| import { connect } from 'react-redux'; | ||
| import { TOCEntry } from './toc_entry'; | ||
| import { TOCEntry } from './view'; |
There was a problem hiding this comment.
Why are you renaming this to view? It is a lot more difficult in the debugger and IDE when every file is called view. Why not name the view files so there is less name collision?
There was a problem hiding this comment.
consistency with namings for our other components that use an index file
There was a problem hiding this comment.
But why not go the other way, and rename all view.js files to aNameThatActuallyMeansSomething.js? Seems like reverting to view.js goes backwards
| }, | ||
| }, | ||
| { | ||
| name: 'Change visibility', |
There was a problem hiding this comment.
The toggle visibility text should be dynamic
this.props.layer.isVisible() ? 'hide layer' : 'show layer'
💔 Build Failed |
nreese
left a comment
nreese
left a comment
There was a problem hiding this comment.
lgtm with green CI
code review, tested changes in chrome
💔 Build Failed |
💚 Build Succeeded |
|
Just one more thing. When the layer is hidden (via the Show/Hide layer button) can you change the layer icon to be the |
|
OH nm, looks like you merged already. |
💚 Build Succeeded |
|
@cchaos too eager... yeah, I can change that to eye-closed.. |
4 participants
Closes #28524
This also refactors the es-search-sources, consolidating shared code, and rename abstract class names for clarity.
in progress, this is just an outline