Skip to content

Responsive Design - Info Box#5396

Closed
vlt1 wants to merge 8 commits intogeosolutions-it:masterfrom
vlt1:c125-info-box-wysiwyg
Closed

Responsive Design - Info Box#5396
vlt1 wants to merge 8 commits intogeosolutions-it:masterfrom
vlt1:c125-info-box-wysiwyg

Conversation

@vlt1
Copy link
Copy Markdown
Contributor

@vlt1 vlt1 commented Jun 3, 2020

Description

Contains austrocontrol-C125/#160 and austrocontrol-C125/#162 since austrocontrol-C125/#162 depends on changes in austrocontrol-C125/#160.
For the former issue #5295 pr has been opened and reviewed. This pr adds new details editor based on Stories editor. Since Stories editor uses an already available wysiwyg editor built on draftJS(react-draft-wysiwyg), that makes it impossible to use plugins mentioned in the issue description, as that would require building a new draftJS based editor from scratch. The editor used in Stories, though, supports image manipulation that I enabled and adapted to the use case. It should be noted that Image manipulation component as well editor's block rendering function can be both overriden with custom components and supplied to react-draft-wysiwyg editor if needs be. For now I just made sure that the already existing functionality works as expected.

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x", remove the others)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

Issue

austrocontrol-C125/#160
austrocontrol-C125/#162

What is the current behavior?
Map details can be edited only through MetadataModal in MapsGrid.

What is the new behavior?
Map details can be edited in MetadataModal as well as in Details map plugin with features as described in austrocontrol-C125/#160. Also a new Details editor based on Stories editor with image manipulation is made available in both MetadataModal and Details plugin.

Breaking change

Does this PR introduce a breaking change? (check one with "x", remove the other)

  • Yes, and I documented them in migration notes
  • No

@vlt1 vlt1 self-assigned this Jun 3, 2020
@vlt1 vlt1 mentioned this pull request Jun 3, 2020
12 tasks
@coveralls
Copy link
Copy Markdown

coveralls commented Jun 3, 2020

Coverage Status

Coverage decreased (-0.06%) to 83.648% when pulling 00d81a1 on vlt1:c125-info-box-wysiwyg into 3e70306 on geosolutions-it:master.

@tdipisa tdipisa added this to the 2020.03.00 milestone Jun 5, 2020
@tdipisa tdipisa requested a review from offtherailz June 22, 2020 11:08
@offtherailz
Copy link
Copy Markdown
Member

Sorry, didn't review it yet. Wrong button hit

@offtherailz offtherailz self-requested a review July 9, 2020 09:26
Copy link
Copy Markdown
Member

@offtherailz offtherailz left a comment

Choose a reason for hiding this comment

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

  • The button should be at least below the spinner, not above.
    image Also a tooltip should be visible.
  • The editor for images has some problems imho: I suggested this approch (see "Alignment + Resize + Focus + Drag'n'Drop + Drag'n'Drop Upload Example") :
    https://www.draft-js-plugins.com/plugin/image .
    Peek 09-07-2020 15-43
    Instead see this:
    Peek 09-07-2020 15-45
    Did I missed something in the communication or installing libs? It looks somehow incomplete (buttons do not work). If we are going to implement draft-js image part in a separate PR, please remove alignment buttons that do not work.
  • The UI to set details option is a little hard to use and understand. You must pass from the home page to create details, then for options set-up you have to open the map. Details are saved automatically when you close the panel and to remove the details you have to go back to the home page. @tdipisa I think we could improve a little bit, do we have a mock-up?
    Peek 09-07-2020 15-57
    Note: in this gif I set up the settings before, on a detail card that I removed, to open on startup. For this reason first time it appears, but by default the behaviour is the current, render in a panel, not open on startup

@offtherailz
Copy link
Copy Markdown
Member

offtherailz commented Jul 9, 2020

About the last point, I sync about it with @tdipisa and we found the following solution:

Create

Create should be possible also from option menu. An entry visible only to the editor allows to create or edit details for the current map. It will open the same window of detail map, in edit mode.
(In containers, if you don't set priority, all items are shown, so you can add an entry for navigation Toolbar and one for BurgerMenu).

Edit

In order to uniform the UI the details saving will appen when the user click on a save button (visible only to the editor) at the bottom.It will save both text and options.
image

Remove

Another button should be visible in the toolbar, if the detail card has been saved, to delete (with a trash icon).

View

The button in navigation toolbar should work as now (with a lower position) in order to allow users in view mode only to see the detail card in view mode.

@vlt1
Copy link
Copy Markdown
Contributor Author

vlt1 commented Jul 9, 2020

The editor for images has some problems imho: I suggested this approch (see "Alignment + Resize + Focus + Drag'n'Drop + Drag'n'Drop Upload Example")

@offtherailz I have addressed this point specifically in the PR description:

Since Stories editor uses an already available wysiwyg editor built on draftJS(react-draft-wysiwyg), that makes it impossible to use plugins mentioned in the issue description, as that would require building a new draftJS based editor from scratch. The editor used in Stories, though, supports image manipulation that I enabled and adapted to the use case. It should be noted that Image manipulation component as well editor's block rendering function can be both overriden with custom components and supplied to react-draft-wysiwyg editor if needs be. For now I just made sure that the already existing functionality works as expected.

Also buttons work, but their functionality is limited. See "Unesco Italian Items" map for an example.

@offtherailz
Copy link
Copy Markdown
Member

mmm... it's ok, but anyway the current image tool is very hard to be used.
@vlt Do you think we can use the base components of draftjs to provide the functionalities I asked (in fact wysiwyg uses draftjs and it's plugins, it looks a sort of preset)?
Maybe a separate issue is the best place to do it?
For the rest of the task, you can proceed as asked.

@vlt1
Copy link
Copy Markdown
Contributor Author

vlt1 commented Jul 29, 2020

in fact wysiwyg uses draftjs and it's plugins

@offtherailz To use plugins one must use Editor component from draft-js-plugins-editor, that actually has an ability to accept plugins. The wysiwyg editor seems to be using Editor from draft-js. The problem it seems to me has two solutions: creating a new editor based on draft-js-plugins-editor from scratch or try to extend the existing functionality of react-wysiwyg-editor either by writing custom components or forking the entire project and making necessary reworks/changes.

@offtherailz
Copy link
Copy Markdown
Member

@vlt1 I think at the end we will need to to something like creating a new edtiror from draft-js, because the image handling from wysiwyg is very hard for a user.
I was only wandering if doing it now or in a separate issue. I prefer the second, please continue with the other tasks. Thank you for your opinion

@vlt1
Copy link
Copy Markdown
Contributor Author

vlt1 commented Jul 29, 2020

@offtherailz If I understood correctly, to sum up:

  • Details panel should be available anytime; if administrator tries to edit and save it will create new details if they don't exist;
  • Toolbar in the details panel should have an additional trash can button to remove the details if they exist;
  • Saving will only be done when user presses Save button, if window is closed a confimation window will show to get a confirmation from the user that they want to close the window without saving;
  • Edit button will have a toggle behaviour;

Everything else stays the same. Correct? Also, what should be done when the user is on a new map? Details are stored as a separate resource and their id is stored as an attribute of the map resource. When the user is working with the new map, that hasn't been saved yet, the map resource doesn't exist so we can't update the attribute. What is the desired behaviour in this case?

@offtherailz
Copy link
Copy Markdown
Member

offtherailz commented Aug 11, 2020

  • Details panel should be available anytime; if administrator tries to edit and save it will create new details if they don't exist;

Yes, if you're allowed to edit it.

  • Toolbar in the details panel should have an additional trash can button to remove the details if they exist;

Yes, good idea. But see my last comment.

  • Saving will only be done when user presses Save button, if window is closed a confimation window will show to get a confirmation from the user that they want to close the window without saving;

Yes... this sometimes is good, sometimes is bad. But I think we should keep this behaviour of explicit save contextual to the map save. Do you agree @tdipisa ?Maybe this should be notified together with the other map changes to avoid the user to leave the map without saving

  • Edit button will have a toggle behaviour;

Yes

Everything else stays the same. Correct? Also, what should be done when the user is on a new map? Details are stored as a separate resource and their id is stored as an attribute of the map resource. When the user is working with the new map, that hasn't been saved yet, the map resource doesn't exist so we can't update the attribute. What is the desired behaviour in this case?

Now that I see this comment, I think we can include your work as a renew of the "Detail sheet" in the save modal, instead of a button of the menu...

What if this tool is accessible from the SaveModal, instead, as for the home page? This can be a finalization for this task:
#2908
I'll provide more details during the week.

@offtherailz
Copy link
Copy Markdown
Member

Ok. I discussed with @tdipisa about this task and we decided to split it in 2 tasks. I'm going to block this PR for the moment to split better the work in different pices:

First of all please work on #5772
This gives you the base entry point to add this editor, with the new options, in save modal instead of the entry of the menu.

Basically the issue replaces the editor-only entry in the Burger Menu option. Everything becomes uniform in home page and map viewer. Then you can add your changes with new option buttons.

note: at the end window to edit the detail card in the home page will be the same in home page and map, with same options on top, and same wysiwyg editor.

note we forked react-wysiwyg, so we can check if we makes sense to improve the editor with the suggestions in this discussion.

@vlt1
Copy link
Copy Markdown
Contributor Author

vlt1 commented Sep 10, 2020

@offtherailz now that the Uniformation is merged, I can proceed with this. Lets iron out the specifics:

  • All editing is done through Save modal in Save plugin with Details sheet component. What is needed now is to add the toggle buttons for options(open on load, open in panel or in dialog). They should be put in the details row alongside the preview button, edit button, etc. or in editor?
  • The Details plugin now will work as a viewer only again, with a capability to open as a dialog or a panel depending on the settings. It should be accessible only through the button in Nav Menu on the top below the spinner:
    image. Or it should also be present in burger menu if Toolbar is not present?
  • Should the editor be the Stories one but without the image manipulation, or the old Quill one?

I will also probably create a new pr from a new branch, it seems easier then merging at this point.

@vlt1 vlt1 mentioned this pull request Sep 11, 2020
12 tasks
@vlt1
Copy link
Copy Markdown
Contributor Author

vlt1 commented Sep 11, 2020

Closing in favor of #5861

@vlt1 vlt1 closed this Sep 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants