Iframes#978 #984

ddfridley · 2023-06-26T21:55:06Z

Also - one issue. The path is /igs instead of /igroups as was in the spec. The problem is that I originally tried to do this as a sitemap - using igroups modeled after the groups path. But I learned it's better to do it as just a path. BUT even after deleting all the code around the /igroups path - it still routs to that path. It seems to be cached somethere that I haven't figures out yet. So I can't change the path to /igroups and get it to run my code. Any ideas?

If you build this code and run it locally, then you can brows to the file ./iframe-test.html and it will load the projects as an iframe:

There might also be a better place to put that test file than root - I'm open to where.

…n a way that can be used in an iframe

ddfridley · 2023-06-28T18:15:20Z

Here's what it looks like embedded in the Open San Diego jekyll page:

And here's the code that it took to embed it there:

        <div style="position: relative; overflow: hidden; width: auto; margin-left: -3rem; margin-right: -3rem; height: 83rem; border-radius:1rem; border: 1px solid black;">
            <iframe
                src="http://localhost:8000/igs/1"
                style="position: absolute; top: 0; left: 0; bottom: 0; right: 0; width: 100%; height: 100%; border: none; border-radius: 1rem;"
            ></iframe>
        </div>

… DEBUG

ddfridley · 2023-07-10T19:56:39Z

iframe resizer has been added and the ips path for rendering projects in an iframe is implemented. This is ready for merging.
But there are two things that have been changed that need to be considered:

The Dockerfile has the changes from what it takes to install gdal for python in docker #982 that are required for me to run this on docker on my machine. I agree, it should not be different on different machines. On the other hand, other's have had trouble with requirements.py solves #985 requirements.txt installation failure #989. I think there is some python or linux version discrepancy. One option is to take out this change from the PR and see if it can work in qa and production - there is already a separate PR for it.
In settings.py I have made the CACHES conditional on the DEBUG environment. Not caching helped with a problem where web pages don't change after I change the code. But perhaps there is another way to do this, or perhaps in QA you need CACEHES enabled and DEBUG.
There are 2 test*.html files in the root directory that maybe should go somewhere else.

I would like to get this into QA at some public URL so I can test for cross-site permissions problems.

ddfridley · 2023-08-08T03:49:50Z

I pulled the latest master, and added opensandiego.org to CSP_FRAME_SRC and CSP_FRAME_ANCESTORS in the democracy-lab-dev heroku settings and then deployed it there. It works, and I tested it with the opensandiego.org page to make sure it works. -- but now I have updated the opensandiego page to point to production so it won't show the incorrect info,

A better resolution to manually setting the CSP_... env variables would be to ask leader to enter the URL of their web site when they create their groups, and projects, and then automatically add those domains to the CSP variables - but for now the number of sites will be small, and we should get this feature out their and get feedback before making it easier to administer.

So I think this is ready to go.

marlonkeating · 2023-08-11T00:37:10Z

Dockerfile

Is this identical to what's in #982?

Yes. It doesn't work for me on windows without this. I think I have gotten a few other people have tried #982 and at least not complained, and had reports of success with the getting started doc too.

marlonkeating · 2023-08-11T00:42:53Z

common/components/controllers/AboutGroupController.jsx

@@ -60,7 +60,7 @@ class AboutGroupController extends React.PureComponent<{||}, State> {

  _renderLoading(): React$Node {
    return this.state.statusCode
-      ? this_.renderLoadErrorMessage()
+      ? this._renderLoadErrorMessage()


Is there any change here?

Yes. It's a bug fix. this_ is undefined and causes an exception when executed. Where as this is what was intended. I encountered this when testing.

Oh, keen eye! I looked over this I don't know how many times and didn't notice...

marlonkeating · 2023-08-11T00:44:38Z

common/components/componentsBySection/FindProjects/ProjectCardsContainer.jsx

@@ -71,7 +71,7 @@ class ProjectCardsContainer extends React.Component<Props, State> {
          </React.Fragment>
        ) : null}
        <div className="row">
-          {!_.isEmpty(this.state.projects) && (
+          {!_.isEmpty(this.state.projects) && !this.props.supressHeader && (


Spelling, and be sure to add to Props type above

Suggested change

{!_.isEmpty(this.state.projects) && !this.props.supressHeader && (

{!_.isEmpty(this.state.projects) && !this.props.suppressHeader && (

marlonkeating · 2023-08-11T01:21:18Z

common/components/common/groups/IframeGroupDisplay.jsx

+import type { GroupDetailsAPIData } from "../../utils/GroupAPIUtils.js";
+import ProjectCardsContainer from "../../componentsBySection/FindProjects/ProjectCardsContainer.jsx";
+
+class IframeGroupDisplay extends AboutGroupDisplay<Props, State> {


Why does this need to inherit from AboutGroupDisplay?

This function needs to have its own unique name, and would have a lot of code duplicated from AboutGroupDisplay, but the render function is different. Doing it this way eliminates duplicate code.

marlonkeating · 2023-08-11T01:23:09Z

common/components/common/projects/AboutProjectDisplay.jsx

@@ -108,8 +108,9 @@ class AboutProjectDisplay extends React.PureComponent<Props, State> {

  render(): React$Node {
    const project = this.state.project;
+    const widthModifier=window.parent !== window ? ' use-parent-width' : '';


The window.parent !== window check is made in a lot of places, it should live in a helper function.

isWithinIframe - done.

marlonkeating · 2023-08-11T01:25:07Z

common/components/componentsBySection/FindProjects/ProjectCard.jsx

+    // iFrameResizer will startup after the page is rendered, so we need to rerener if it does
+    if(!window.iFrameResizer) window.iFrameResizer={}
+    if(!window.iFrameResizer.onInParent) window.iFrameResizer.onInParent=[]
+    window.iFrameResizer.onInParent.push(this.onIframeResizer.bind(this))


This should live in a helper function or wrapper component.
Also, don't forget line semicolons!

This was an interesting challenge. I created common/components/common/IframeResizerInParent.jsx a wrapper component with static helper functions that cleanly factored this out.

marlonkeating · 2023-08-11T01:27:49Z

common/components/controllers/MainController.jsx

+      if(!window.iFrameResizer) window.iFrameResizer={}
+      window.iFrameResizer.inParent=true
+      let func
+      while((func=window.iFrameResizer.onInParent?.shift()))


What is this line doing?

I will add a comment to the code. There could be multiple components needing to be re-rendered after iframe resizer starts, so onInParent is an array of callbacks from those components to run after iframeresizer starts. Components may need to rerender after iframeresizer starts because their style needs to change if rendering in an iframe that resizes v. rendering in an iframe that doesn't resize.

marlonkeating · 2023-08-11T01:30:21Z

common/components/urls/urls_v2.json

@@ -6,6 +6,7 @@
  {"name": "CreateEventProject", "pattern": "events/(?P<event_id>[\\w\\-]+)/projects/create/(?P<project_id>[\\w\\-]*)"},
  {"name": "AboutEventProject", "pattern": "events/(?P<event_id>[\\w\\-]+)/projects/(?P<project_id>[\\w\\-]+)"},
  {"name": "CreateProject", "pattern": "projects/create/(?P<id>[\\w\\-]*)"},
+  {"name": "IframeProject", "pattern": "ips/(?P<id>[\\w\\-]+)"}, 


My preference would be for urls of iframe versions of pages to branch off from the main urls.

Suggested change

{"name": "IframeProject", "pattern": "ips/(?P<id>[\\w\\-]+)"},

{"name": "IframeProject", "pattern": "projects/ips/(?P<id>[\\w\\-]+)"},

done but its projects/inframe/ and groups/inframe/

marlonkeating · 2023-08-11T01:32:34Z

iframe-test-resizer.html

Is this a demonstration file for clients wanting to embed projects/groups? If so, add comments.
Also, these html files should probably live in a separate folder.

These two files are intended for use be developers in unit testing. I couldn't figure out a directory for putting them in. But they should be retained for future testing of this feature. And I will put comments in them explaining that's what the are for.

But how would you feel about a tools directory at the top level and I would put them in there. It's doesn't feel right to call it a test directory because that kind of implies automated tests. Just leaving them at the root isn't very clean. Or I am open to other ideas.

I created a tools directory and put them there.

ddfridley · 2023-08-11T19:09:49Z

Okay I've made and pushed the changes and this is ready to go.

marlonkeating · 2023-09-03T22:51:43Z

common/components/controllers/MainController.jsx

-      {ShowHeadAndFoot && <SponsorFooter key="sponsor_footer" />}
-      {ShowHeadAndFoot && <SiteFooter key="site_footer" />}
-    </>
+    const ShowHeadAndFoot=!(window.location.pathname.includes('/groups/inframe')||window.location.pathname.includes('/projects/inframe'));


This statement is used a couple times, so should live in a helper method. Can also use helper methods to implement the logic.

Suggested change

const ShowHeadAndFoot=!(window.location.pathname.includes('/groups/inframe')||window.location.pathname.includes('/projects/inframe'));

const ShowHeadAndFoot=!(urlHelper.atSection(Section.IframeProject)||urlHelper.atSection(Section.IframeGroup));

I made the change to urlHelper both in this file and in ProjectCard. - but the code is different between the two and a common helper method wasn't possible. Plus both places read well with the urlHelper change. The ProjectCard code looks like this now:

const url: string = (urlHelper.atSection(Section.IframeGroup) && IframeResizerInParent.inParent()) ? urlHelper.section(Section.IframeProject, { id: this.props.project.slug || this.props.project.id, }) : ( this.props.project.cardUrl || urlHelper.section(Section.AboutProject, { id: this.props.project.slug || this.props.project.id, }) );

marlonkeating · 2023-09-03T22:58:13Z

common/components/utils/isWithinIframe.js

nit: I'd rename this file as just iframe.js so we can have a util file dedicated to iframes.

marlonkeating · 2023-09-03T23:07:50Z

common/components/common/projects/IframeProjectDisplay.jsx

Do we need this file? It seems to be just a wrapper around AboutProjectDisplay (which handles the iframe logic internally).

removed it and IframeProjectDisplay by the same logic.

ddfridley · 2023-09-06T05:17:06Z

I've tested this locally, and pushed it to democracy-lab-dev and am testing it a little more there, but won't finish until tomorrow.

ddfridley · 2023-09-06T18:05:16Z

ok, I'm done with my testing. ready to go.

ddfridley · 2023-09-06T18:07:39Z

Also, please add opensandiego.org to CSP_FRAME_SRC and CSP_FRAME_ANCESTORS in the environment settings after you push to production.

ddfridley · 2023-09-06T19:30:52Z

More specifically in democracy-lab-dev I have:

CSP_FRAME_ANCESTORS=["qiqochat.com", "*.qiqochat.com", "*.google.com", "democracylab.org", "democracy-lab-prod-mirror.herokuapp.com", "democracy-lab-dev.herokuapp.com", "democracy-lab-staging.herokuapp.com", "*.doubleclick.net","https://opensandiego.org/"]
CSP_FRAME_SRC=["qiqochat.com", "*.qiqochat.com", "*.google.com", "*.youtube.com", "democracylab.org", "democracy-lab-prod-mirror.herokuapp.com", "democracy-lab-dev.herokuapp.com", "democracy-lab-staging.herokuapp.com", "*.doubleclick.net", "*.hotjar.com","*.opensandiego.org"]

I remember it didn't work without the https:// in front for CSP_FRAME_ANCESTORS

PeterBreen

I've made some comments on testing issues I've found, mostly on very small or very large devices. It's not exhaustive so please be sure to take a look at the whole layout on any test deployments (e.g. open san diego) on the whole range of small to very large viewports -- I'm sure there's going to be issues I didn't encounter using the test html files provided.

Broadly, dlab's site when displayed natively supports a mobile minimum of 360px device width up to a maximum viewport of 1440px (at which point the content is 1392px width and does not expand further) -- so please use those as your test boundaries for displaying dlab in a frame.

civictechprojects/static/css/partials/_Profile.scss

tools/iframe-test-resizer.html

tools/iframe-test.html

tools/iframe-test-resizer.html

tools/iframe-test.html

ddfridley

I have 3 pending comments so I am doing this to submit them.
With this new push, I think this is ready to go.

tools/iframe-test-resizer.html

tools/iframe-test.html

PeterBreen · 2023-09-17T20:48:15Z

Approved all the parts of my review. Anything from your review, Marlon, I left alone.

ddfridley · 2023-09-17T21:47:22Z

@marlonkeating I should have thought of this earlier. I create a test server on heroku to use the iframe feature.
Please add
"https://dlab-iframe-test-0ac0b57c4ba3.herokuapp.com/" to the end of CSP_FRAME_ANCESTORS
and
"*.dlab-iframe-test-0ac0b57c4ba3.herokuapp.com/" to the end of CSP_FRAME_SRC
in the production-mirror environment.
Note that both those variables are arrays and it needs to fit the syntax.
[I don't know why I'm getting uuids on the urls now, but I guess they want to make it harder for the ECO servers"]
Should we add this to the production environment as well?
And remember in production we need to add opensandiego.org

This will test the feature: https://dlab-iframe-test-0ac0b57c4ba3.herokuapp.com/

ddfridley · 2023-09-17T22:40:13Z

update to above:
I pushed this branch to democracy-lab-dev and did the environment changes and testing there.

marlonkeating · 2023-10-04T16:41:54Z

common/components/utils/url.js

      let _args: Dictionary<string> = {
-        prev: sectionArgs.section,
+        prev: sectionArgs.section!==Section.IframeProject ? sectionArgs.section : Section.AboutProject, // if login from within an iframe, go direct without iframe


Do we want to have the same logic for groups?

Also, be sure to add new logInThenReturn unit tests to

CivicTechExchange/common/components/test/utils.test.js

Line 157 in 3f69856

test("logInThenReturn produces correct redirection urls", () => {

C:/Program Files/Git/igs/1 will render just the projects of a group i…

e80f303

…n a way that can be used in an iframe

ddfridley requested review from marlonkeating and PeterBreen June 26, 2023 21:55

marlonkeating linked an issue Jul 6, 2023 that may be closed by this pull request

Embedding DemocracyLab groups and projects in other websites as iframes #978

Closed

4 tasks

projects work in iframes, iframe-resizer works, CACHES is disabled in…

d72bfce

… DEBUG

ddfridley marked this pull request as ready for review July 10, 2023 19:56

marlonkeating temporarily deployed to democracy-lab-prod-mirror August 2, 2023 23:33 Inactive

merged with master - kept changes to settings.py

7fd27ac

marlonkeating temporarily deployed to democracy-lab-prod-mirror August 3, 2023 04:52 Inactive

Merge remote-tracking branch 'origin/master' into iframes#978

b6ac4d7

ddfridley temporarily deployed to democracy-lab-dev August 8, 2023 03:23 Inactive

marlonkeating requested changes Aug 11, 2023

View reviewed changes

ddfridley added 4 commits August 11, 2023 09:09

move test helper files to tools dir

a00120f

factor isWithinIframe and change path to /inframe

ad58ed0

spelling of suppressHeader

c1d3b23

IframeResizerInParent React Component factored

b7936b1

Merge remote-tracking branch 'origin/master' into iframes#978

0be2a23

marlonkeating requested changes Sep 3, 2023

View reviewed changes

marlonkeating and others added 4 commits September 3, 2023 17:42

Merge branch 'master' into iframes#978

017db42

urlHelper instead of window.location

fe1040e

util/iframe.js is collection of iframe utilities

c2ca756

removed redundant IFrameProjectController and ...Display

dab7704

ddfridley temporarily deployed to democracy-lab-dev September 5, 2023 23:13 Inactive

PeterBreen mentioned this pull request Sep 14, 2023

added gtags events to buttons tabs anchors #1041

Closed

PeterBreen requested changes Sep 14, 2023

View reviewed changes

fix size of projecs in iframes on small and large

c3fafd2

ddfridley commented Sep 15, 2023

View reviewed changes

tools/iframe-test-resizer.html Show resolved Hide resolved

tools/iframe-test.html Show resolved Hide resolved

tools/iframe-test.html Outdated Show resolved Hide resolved

ddfridley temporarily deployed to democracy-lab-dev September 15, 2023 23:35 Inactive

ddfridley and others added 2 commits September 16, 2023 16:28

.override-breakpoint-max-width

53cdb7f

Merge branch 'master' into iframes#978

3f69856

marlonkeating temporarily deployed to democracy-lab-prod-mirror September 17, 2023 16:47 Inactive

PeterBreen previously approved these changes Sep 17, 2023

View reviewed changes

ddfridley temporarily deployed to democracy-lab-dev September 17, 2023 21:53 Inactive

ddfridley added 3 commits September 26, 2023 12:42

base target in iframed project

6d7500f

Merge remote-tracking branch 'origin/master' into iframes#978

d86ec50

Merge remote-tracking branch 'origin/iframes#978' into iframes#978

7515ce6

ddfridley dismissed PeterBreen’s stale review via 7515ce6 September 26, 2023 19:50

ddfridley temporarily deployed to democracy-lab-dev September 26, 2023 19:53 Inactive

login return to non iframed about project

399640b

ddfridley temporarily deployed to democracy-lab-dev September 28, 2023 18:55 Inactive

marlonkeating requested changes Oct 4, 2023

View reviewed changes

Unit test for project iframe redirect

1c598ff

marlonkeating temporarily deployed to democracy-lab-prod-mirror October 9, 2023 16:44 Inactive

marlonkeating temporarily deployed to democracy-lab October 10, 2023 05:39 Inactive

marlonkeating approved these changes Oct 15, 2023

View reviewed changes

marlonkeating merged commit bf42a76 into master Oct 15, 2023

	{!_.isEmpty(this.state.projects) && !this.props.supressHeader && (
	{!_.isEmpty(this.state.projects) && !this.props.suppressHeader && (

	{"name": "IframeProject", "pattern": "ips/(?P<id>[\\w\\-]+)"},
	{"name": "IframeProject", "pattern": "projects/ips/(?P<id>[\\w\\-]+)"},

	const ShowHeadAndFoot=!(window.location.pathname.includes('/groups/inframe')\|\|window.location.pathname.includes('/projects/inframe'));
	const ShowHeadAndFoot=!(urlHelper.atSection(Section.IframeProject)\|\|urlHelper.atSection(Section.IframeGroup));

Iframes#978 #984

Iframes#978 #984

Conversation

ddfridley commented Jun 26, 2023 • edited

ddfridley commented Jun 28, 2023

ddfridley commented Jul 10, 2023

ddfridley commented Aug 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddfridley commented Aug 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddfridley commented Sep 6, 2023

ddfridley commented Sep 6, 2023

ddfridley commented Sep 6, 2023

ddfridley commented Sep 6, 2023

PeterBreen left a comment

Choose a reason for hiding this comment

ddfridley left a comment

Choose a reason for hiding this comment

PeterBreen commented Sep 17, 2023

ddfridley commented Sep 17, 2023

ddfridley commented Sep 17, 2023

update to above: I pushed this branch to democracy-lab-dev and did the environment changes and testing there.

Choose a reason for hiding this comment

ddfridley commented Jun 26, 2023 •

edited

update to above:
I pushed this branch to democracy-lab-dev and did the environment changes and testing there.