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

Jump to bottom

Focus preview even If website fetch url doesn't exist #9289

Merged
merged 2 commits into from Oct 18, 2020
Merged

Focus preview even If website fetch url doesn't exist #9289

merged 2 commits into from Oct 18, 2020

Conversation

kuzmany
Copy link
Member

@kuzmany kuzmany commented Oct 12, 2020

Q A
Branch? staging for features or enhancements / 3.0 for bug fixes
Bug fix? yes/no
New feature? yes/no
Deprecations? yes/no
BC breaks? yes/no
Automated tests included? yes/no
Related user documentation PR URL mautic/mautic-documentation#...
Related developer documentation PR URL mautic/developer-documentation#...
Issue(s) addressed Fixes #...

Description:

PR #8894 rework the preview of Focus with iframe. It works properly, but preview display just If website url to fetch exists.
But in Mautic 2 preview works even If website URL not exists.
This is confusing for migrate instances from M2 to M3.
This PR allow generate preview even If url does not exists.

Steps to test this PR:

  1. Load up this PR
  2. Create focus without URL.
  3. Check If preview is generated
  4. Also check If preview is generated after re-open focus builder.

@kuzmany kuzmany added bug Issues or PR's relating to bugs ready-to-test PR's that are ready to test labels Oct 12, 2020
@kuzmany kuzmany added this to the 3.2 milestone Oct 12, 2020
@cla-bot cla-bot bot added the cla-signed The PR contributors have signed the contributors agreement label Oct 12, 2020
@npracht npracht added the focus-items Anything related to focus items label Oct 12, 2020
@npracht npracht modified the milestones: 3.2, 3.1.2 Oct 12, 2020
mabumusa1
mabumusa1 approved these changes Oct 16, 2020
View reviewed changes
Copy link
Member

@mabumusa1 mabumusa1 left a comment

Choose a reason for hiding this comment

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

Tested the code locally, it worked as expected

@mabumusa1 mabumusa1 added pending-test-confirmation PR's that require one test before they can be merged and removed ready-to-test PR's that are ready to test labels Oct 16, 2020
dennisameling
dennisameling approved these changes Oct 17, 2020
View reviewed changes
Copy link
Member

@dennisameling dennisameling left a comment

Choose a reason for hiding this comment

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

Works as described

@dennisameling dennisameling added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged and removed pending-test-confirmation PR's that require one test before they can be merged labels Oct 17, 2020
@dennisameling dennisameling changed the base branch from staging to 3.1 October 17, 2020 18:44
@npracht npracht merged commit 31f9ec5 into mautic:3.1 Oct 18, 2020
@RCheesley RCheesley added the hacktoberfest-accepted PR's that have been accepted for the purposes of Hacktoberfest label Oct 19, 2020
@nickveenhof
Copy link
Contributor

Tried this but have not successfully been able to confirm this fixes anything. Could also be that it wasn't clear what it exactly fixes.

@dennisameling
Copy link
Member

dennisameling commented Oct 19, 2020 •
edited

@nickveenhof

  • Create a new focus item
  • Make sure you leave the "website" field empty:

image

  • Open the Builder
  • Select "display a notice" and select "modal". You'll notice that the preview screen on the left doesn't show anything:

image

  • Checkout this PR
  • Try again, you should see the preview work correctly now:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mabumusa1 mabumusa1 mabumusa1 approved these changes

@dennisameling dennisameling dennisameling approved these changes

Assignees
No one assigned
Labels
bug Issues or PR's relating to bugs cla-signed The PR contributors have signed the contributors agreement focus-items Anything related to focus items hacktoberfest-accepted PR's that have been accepted for the purposes of Hacktoberfest ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged
Projects
None yet
Milestone
3.1.2
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@kuzmany @nickveenhof @dennisameling @mabumusa1 @RCheesley @npracht