Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

Render Appropriately in Embedded Mode #1260

Merged
merged 3 commits into from Mar 3, 2021
Merged

Conversation

TaiWilkin
Copy link
Contributor

@TaiWilkin TaiWilkin commented Feb 25, 2021

Overview

When OAR is acting as an embedded map, various components should be hidden or behave differently.

Page

  • Remove header and footer

Sidebar:

  • Remove contributor filter
  • Remove contributor type filter
  • Remove report link.
  • Remove Guide tab
  • Remove the "The open map of..." Header
  • Remove actions: Claim, Suggest an Edit, Report as Closed and replace with 'view in OAR' link (target=)
  • Make contributor links plain text

Connects #1256

Demo

Screen Shot 2021-02-23 at 1 56 18 PM

Screen Shot 2021-02-23 at 1 11 48 PM

Testing Instructions

  • Run ./scripts/server in vagrant
  • Navigate to http://localhost:6543/
    • The site should behave as before
  • Navigate to http://localhost:6543/?contributors=8&embed=1
  • Navigate to http://localhost:6543/?contributors=8&embed=1
    • The page header and footer should not be displayed
    • The contributor and contributor type filters should not be displayed
    • The guide tab should not be displayed
    • The sidebar OAR header should not be displayed
    • The ‘report a problem’ link should not be displayed
  • Click the ’share search’ button
    • The current page link with the embed code removed should be added to the clipboard
  • Navigate to a facility
    • The contributor names should be plain text
    • The action links (claim, suggest an edit, report as closed) should be replaced with a single ‘view in OAR’
  • Click the ‘view in OAR’ link
    • It should open the same page in a new tab without the ‘embed’ code

Checklist

  • fixup! commits have been squashed
  • CI passes after rebase
  • CHANGELOG.md updated with summary of features or fixes, following Keep a Changelog guidelines

@jwalgran
Copy link
Contributor

Looking at this now.

Copy link
Contributor

@jwalgran jwalgran left a comment

Choose a reason for hiding this comment

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

Nice and complete implementation of a "laundry list" feature with lots of concerns. 👍

src/app/src/App.jsx Show resolved Hide resolved
src/app/src/components/FacilityDetailSidebar.jsx Outdated Show resolved Hide resolved
src/app/src/components/FilterSidebarFacilitiesTab.jsx Outdated Show resolved Hide resolved
Report an issue
</a>
{embed ? (
<div />
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious about this empty div. Traditionally we return null from our ternary expressions to skip rendering a component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to maintain styling - having no component here caused the search/reset buttons to shift left. I could rewrite the styling code explicitly instead, if desired

Copy link
Contributor

Choose a reason for hiding this comment

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

2 cents: As a rule of thumb, best to avoid spacer divs and make the styling resilient to (expected) DOM variations. But not a huge deal here, and it should disappear anyway when we do a more holistic update to the search form to accommodate additional functionality.

src/app/src/util/util.js Outdated Show resolved Hide resolved
@jwalgran jwalgran assigned TaiWilkin and unassigned jwalgran Feb 25, 2021
@lederer
Copy link
Contributor

lederer commented Feb 26, 2021

👀

@lederer
Copy link
Contributor

lederer commented Feb 26, 2021

Heads up, I'm seeing a console warning when I open the facility detail view – both in embed and regular mode. But it's also present on develop so I reckon it's out of scope.

Failed prop type: The prop `children` is marked as required in `FeatureFlag`, but its value is `null`.
    in FeatureFlag (created by Connect(FeatureFlag))
    in Connect(FeatureFlag) (at FacilityDetailSidebar.jsx:360)
    in div (at FacilityDetailSidebar.jsx:359)
    in div (at FacilityDetailSidebar.jsx:298)
...

Copy link
Contributor

@lederer lederer 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.

@TaiWilkin TaiWilkin force-pushed the tw/hide-components-embedded branch 2 times, most recently from 3922ad5 to ac3f8c8 Compare March 2, 2021 17:45
@jwalgran jwalgran mentioned this pull request Mar 3, 2021
15 tasks
When running in embedded mode, hide the header and footer,
the contributor filter, the report link, the Guide tab, the OAR sidebar
header, and the facility detail actions; make the contributor links into
 text; add a 'open in OAR' link; and remove the 'embed' parameter from
 'share this search'.
@TaiWilkin TaiWilkin force-pushed the tw/hide-components-embedded branch from ac3f8c8 to e62483e Compare March 3, 2021 21:33
@TaiWilkin TaiWilkin merged commit b95d127 into develop Mar 3, 2021
@TaiWilkin TaiWilkin deleted the tw/hide-components-embedded branch March 3, 2021 21:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants