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

Fix: Implement PDF preview inside dashboards #9698

Conversation

mumairr
Copy link
Contributor

@mumairr mumairr commented Nov 13, 2023

Description

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

What is the current behavior?

Dashboards use React Quill Editor

What is the new behavior?

Implements Draftjs based editor for dashboards (Text) to allow embedding of any external link, document on the widget.
Fixes up scroll issue on widgets created from map

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

Other useful information

This PR is for backport based on version 2022.02.xx for use in GN. Same for the master has already been implemented last week,

https://github.com/geosolutions-it/C273-ISPRA-2023-UNEP-MAP/issues/42

@mumairr mumairr self-assigned this Nov 13, 2023
@mumairr mumairr added Backport New Feature used for new functionalities new and removed new labels Nov 13, 2023
@giohappy giohappy requested a review from dsuren1 November 20, 2023 08:32
Copy link
Contributor

@dsuren1 dsuren1 left a comment

Choose a reason for hiding this comment

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

@mumairr
Kindly amend these minor style issues. Thanks!
image

@@ -30,7 +30,7 @@


.ms-wizard {
position: absolute;
// position: absolute;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is needed. Kindly check and revert this change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dsuren1 this was needed to solve a small bug, it came into limelight after Ellano's comment #2 on my previous PR here
Interestingly this came into play as well for this backport. If we add up a long text, links etc. in editor for geostories via map, scrollbar doesn't appear. Kindly check it

Copy link
Contributor

@dsuren1 dsuren1 Nov 21, 2023

Choose a reason for hiding this comment

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

Scrollbar is present with position retained as is for ms-wizard
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It occurs only if you add dashboard txt widget via map for a geostory. I was also confused with it and Stefano guided me how to reproduce it.

image
image

@mumairr
Copy link
Contributor Author

mumairr commented Nov 21, 2023

@mumairr Kindly amend these minor style issues. Thanks! image

Fixing styles for this one

@dsuren1
Copy link
Contributor

dsuren1 commented Nov 21, 2023

@mumairr
I'm seeing this too but not in DEV. Looks like some styles needs to be adopted from master
image

@dsuren1
Copy link
Contributor

dsuren1 commented Nov 21, 2023

@mumairr
Looks like there is an open PR of the original issue with some fixes. Hence the PR #9696 has be accepted and tested in DEV before you can make any backport of this feature to 2022.02.xx. So kindly stop making any further changes here until #9696 is merged and tested

@mumairr
Copy link
Contributor Author

mumairr commented Nov 21, 2023

@mumairr Looks like there is an open PR of the original issue with some fixes. Hence the PR #9696 has be accepted and tested in DEV before you can make any backport of this feature to 2022.02.xx. So kindly stop making any further changes here until #9696 is merged and tested

right @dsuren1. I will keep it on hold

@allyoucanmap
Copy link
Contributor

Closing because has been solved here #9724

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backport New Feature used for new functionalities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants