Improve layout in the map viewer #11789
Conversation
…at/change-map-layout
There was a problem hiding this comment.
In addition to the inline comments:
- Ensure the configuration for Map and FeatureEditor position works for existing contexts. We probably need to add a new statement here https://github.com/geosolutions-it/MapStore2/blob/master/web/client/utils/ContextCreatorUtils.js#L27-L61. Mow if you open this existing context #/context/demo_context/46500 the position of attribute table is wrong
- Include unit tests
| import React from "react"; | ||
| import FlexBox, { FlexFill } from "./FlexBox"; | ||
|
|
||
| export default ({ |
There was a problem hiding this comment.
please avoid direct default export, write instead
const MapViewerLayout = () => ({
...
export default MapViewerLayout;| <FlexBox id={id} className={className} column classNames={['_fill', '_absolute']}> | ||
| {header} | ||
| <FlexFill flexBox column className={bodyClassName} classNames={['_relative', 'ms2-layout-body']}> | ||
| <div className="_fill _absolute">{background}</div> | ||
| <div className="_relative">{top}</div> | ||
| <FlexFill flexBox classNames={['_relative', 'ms2-layout-main-content']}> | ||
| <div className="_relative ms2-layout-left-column">{leftColumn}</div> | ||
| <FlexFill classNames={['_relative', 'ms2-layout-content']}> | ||
| {children} | ||
| </FlexFill> | ||
| <div className="_relative ms2-layout-right-column">{rightColumn}</div> | ||
| <div className="ms2-layout-columns">{columns}</div> | ||
| </FlexFill> | ||
| <div className="_relative">{bottom}</div> | ||
| </FlexFill> | ||
| {footer} | ||
| </FlexBox> |
There was a problem hiding this comment.
Let's change name of the classes to ensure it is not the same of BorderLayout. We could use ms-map-viewer-layout, ms-map-viewer-layout-...
| /* Disable pointer events on the main content container but enable them for all its children */ | ||
| .ms2-layout-main-content { | ||
| pointer-events: none; | ||
| } | ||
| .ms2-layout-main-content .ms2-layout-content > *, | ||
| .ms2-layout-main-content .ms2-layout-left-column > *, | ||
| .ms2-layout-main-content .ms2-layout-right-column > *, | ||
| .ms2-layout-main-content .ms2-layout-columns > * { | ||
| pointer-events: auto; | ||
| } |
There was a problem hiding this comment.
review classes based on previous comment. In addition we should ensure these classes are included in the main theme, the viewer.css may be included in component not used inside other project, I think we should move this inside the .less files, maybe in common.less
| newFeatures: [], | ||
| features: [], | ||
| dockSize: 0.35, | ||
| dockSize: 0, |
There was a problem hiding this comment.
We need to review how dockSize has been used and in case remove completely the logic from MapStore including related actions. Could you check if we had previously logic to set the initial size of the Attribute table panel?
We need also to clean up all the logic related to the feature editor dock bottom size in updateMapLayoutEpic, it's not needed anymore
| style={{ | ||
| height: '4px', | ||
| cursor: 'row-resize', | ||
| backgroundColor: 'transparent', | ||
| position: 'absolute', | ||
| top: 0, | ||
| left: 0, | ||
| right: 0, | ||
| zIndex: 10, | ||
| borderTop: '2px solid transparent', | ||
| transition: 'border-color 0.2s' | ||
| }} | ||
| onMouseEnter={(e) => { | ||
| e.currentTarget.style.borderTopColor = '#ccc'; | ||
| }} | ||
| onMouseLeave={(e) => { | ||
| if (!isResizing) { | ||
| e.currentTarget.style.borderTopColor = 'transparent'; | ||
| } | ||
| }} |
There was a problem hiding this comment.
Could we move all of this in a less or css? We could also use a direct import but I think it would be better to use the the less/css option for the following reasons:
- use of css variable for styling
- use of pseudoselector to apply border color (e.g.: :hover instead of onMouseEnter/onMouseLeave)
| if (isResizing) { | ||
| document.addEventListener('mousemove', handleMouseMove); | ||
| document.addEventListener('mouseup', handleMouseUp); | ||
| document.body.style.cursor = 'row-resize'; | ||
| document.body.style.userSelect = 'none'; | ||
| } | ||
|
|
||
| return () => { | ||
| document.removeEventListener('mousemove', handleMouseMove); | ||
| document.removeEventListener('mouseup', handleMouseUp); | ||
| document.body.style.cursor = ''; | ||
| document.body.style.userSelect = ''; | ||
| }; |
There was a problem hiding this comment.
Maybe it's better to use pointer events instead? pointermove pointerup
| <FlexFill classNames={['_relative', 'ms2-layout-content']}> | ||
| {children} | ||
| </FlexFill> |
There was a problem hiding this comment.
This component container represent the visible content on top of the map, when this component resize we should update also the state of the boundingMapRect of the maplayout reducers. We need to:
- include a callback inside this component that return information about this component changes (we could use a resize observer). This component should not be directly connected to the state
- later we will connect this component when we import it in the MapViewer container
- at the moment we need just to update information for the bottom side, left and right are still managed by the updateMapLayout epic
2 participants
Description
This PR updates the layout of the map viewer with the possibility of adding the map as the background and letting all the components act as the overlay over the map. The feature grid is added in the bottom container of the map layout.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x", remove the others)
Issue
What is the current behavior?
The feature grid is in the dock container of the map viewer.
#11794
What is the new behavior?
The feature grid is displayed in the bottom container of the map layout.
Breaking change
Does this PR introduce a breaking change? (check one with "x", remove the other)