Conversation
There was a problem hiding this comment.
Hi @vlt1 i think you did a good work, but is something i would like to change / improve :
- there is a bug when i try to edit the content the details from in map as modal,
it works if i edit settings
StandardRouter.jsx:130 Error: You are passing the `delta` object from the `onChange` event back as `value`. You most probably want `editor.getContents()` instead. See: https://github.com/zenoamaro/react-quill#using-deltas
at Object.Quill_componentWillReceiveProps [as componentWillReceiveProps] (component.js:168)
at callComponentWillReceiveProps (react-dom.development.js:14329)
at updateClassInstance (react-dom.development.js:14546)
at updateClassComponent (react-dom.development.js:18357)
at beginWork$1 (react-dom.development.js:20108)
at HTMLUnknownElement.callCallback (react-dom.development.js:362)
at Object.invokeGuardedCallbackDev (react-dom.development.js:411)
at invokeGuardedCallback (react-dom.development.js:466)
at beginWork$$1 (react-dom.development.js:25730)
at performUnitOfWork (react-dom.development.js:24638)
at workLoopSync (react-dom.development.js:24614)
at performSyncWorkOnRoot (react-dom.development.js:24182)
at eval (react-dom.development.js:12238)
at unstable_runWithPriority (scheduler.development.js:815)
at runWithPriority$2 (react-dom.development.js:12188)
at flushSyncCallbackQueueImpl (react-dom.development.js:12233)
- ui change
Also i would propose a little change on the ui of the modal to make it equal to the one in the side panel
this is the current situation for the modal
and for the panel
My proposal is:
- to remove the buttons in the footer of the modal and add a toolbar in the header, as done for the panel
- since settings are two checkboxes we can add two toggle button in the new toolbar
- the edit text can be replaced by a pencil icon
- the toolbar will have toolitps and will be centered and always visible (both in view and edit mode)
- when in edit mode there will be a arrow-left and a floppy disk icon.
- this would require some sort of save on exit behaviour if one toggles the settings
@allyoucanmap what do you think ?
| icon: <Glyphicon glyph="sheet"/>, | ||
| action: openDetailsPanel, | ||
| action: setControlProperty.bind(null, 'details', 'enabled', true), | ||
| selector: (state) => { |
There was a problem hiding this comment.
here i have some doubts.
if someone removes the Toolbar from localConfig you are not able to reopen the details if
showAsModal is turned on && showOnStartUp is turned off
In my opinion this plugin should be shown without looking at showAsModal flag.
Another things is that if you add a priority of 1 here and on let's say 5 on the Toolbar container this will make the details plugin visible in the toolbar if both are present, or visible in burger menu if toolbar is not present. (but we still just check the presence of detailsUri
@tdipisa what do you think?
web/client/plugins/Details.jsx
Outdated
| const mapId = mapIdSelector(state); | ||
| const detailsUri = mapId && mapInfoDetailsUriFromIdSelector(state, mapId); | ||
| const settings = settingsSelector(state) || {}; | ||
| if (detailsUri && !settings.showAsModal) { |
There was a problem hiding this comment.
Following my previous post
| if (detailsUri && !settings.showAsModal) { | |
| if (detailsUri) { |
web/client/plugins/Details.jsx
Outdated
| Toolbar: { | ||
| name: 'details', | ||
| position: 1, | ||
| priority: 1, |
There was a problem hiding this comment.
| priority: 1, | |
| priority: 4, |
| text: <Message msgId="details.title"/>, | ||
| icon: <Glyphicon glyph="sheet"/>, | ||
| action: openDetailsPanel, | ||
| action: setControlProperty.bind(null, 'details', 'enabled', true), |
There was a problem hiding this comment.
| action: setControlProperty.bind(null, 'details', 'enabled', true), | |
| priority: 1, | |
| action: setControlProperty.bind(null, 'details', 'enabled', true), |
web/client/plugins/Details.jsx
Outdated
| const detailsUri = mapId && mapInfoDetailsUriFromIdSelector(state, mapId); | ||
| if (detailsUri) { | ||
| const settings = settingsSelector(state) || {}; | ||
| if (detailsUri && settings.showAsModal) { |
There was a problem hiding this comment.
I think showAsModal should not be necessary
| if (detailsUri && settings.showAsModal) { | |
| if (detailsUri) { |
|
@MV88 I couldn't reproduce the bug, could you please provide more information? |
|
@allyoucanmap these changes are not for Details plugin, this is a bugfix connected to changes introduced for MapExport plugin |
i was proposing to remove showAsModal in the condition of selectors The bug was to open a map with details, from within the map, try to save new details,
|
@vlt1 If not strictly related or required for this issue we should remove them and open another issue explaining the problem with MapExport. We usually have to adress one issue for PR, see PR guideline:
|
|
Closing this in favor of #5396. |
Description
Initial Info Box implementation.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x", remove the others)
Issue
austrocontrol-C125/#160
What is the current behavior?
Access to map details in viewer is read-only in panel.
What is the new behavior?
Enhances Details plugin for it to present information either in panel or in dialog form, adds an ability to edit details in viewer, show details information automatically when the map is opened.
Breaking change
Does this PR introduce a breaking change? (check one with "x", remove the other)