mitodl / ocw-hugo-themes

A Hugo theme for building OCW websites
BSD 3-Clause "New" or "Revised" License
5 stars 4 forks source link

feat: add off-site navigation confirmation dialog #1269

Closed HussainTaj-arbisoft closed 8 months ago

HussainTaj-arbisoft commented 1 year ago

What are the relevant tickets?

Closes https://github.com/mitodl/hq/issues/1812

Description (What does it do?)

This PR:

Choice of approach

A single modal's markdown is placed inside the body tag. The confirm action link is dynamically updated when a user clicks an external link.

Bootstrap's modals are sensitive to positioned parents. This is the reason why the modal needs to be placed inside the body tag.

Design: The off-bits

You might observe that the output looks a bit different from the designs (which might not be obvious). The reasons for this are:

The following image shows a stress test of the icon placement design.

Screenshot 2023-10-23 at 1 28 08 PM

Screenshots (if appropriate):

Screenshot 2023-10-23 at 2 53 02 PM Screenshot 2023-10-23 at 2 53 14 PM

https://github.com/mitodl/ocw-hugo-themes/assets/71316217/bfc87c2c-2ea7-4ab6-ab0f-9141c11be6a2

https://github.com/mitodl/ocw-hugo-themes/assets/71316217/82ec1ab8-dd69-4a01-9aa4-c676152f5535

How can this be tested?

  1. Grab content for a course from this list.
  2. Place the content in your local content directory.
  3. Navigate to your local setup of ocw-hugo-themes.
  4. Checkout branch hussaintaj/1812-external-link-modal.
  5. Run
    yarn start course <course-id>
  6. Observe the external links in the sidebar.
    • Expect external links to be marked with an icon.
    • Expect a dialog to pop-up when you click on an external link.
  7. Observe the confirmation dialog.
    • Expect that clicking "Stay Here", the close button, or outside the modal closes the dialog.
    • Expect that clicking on "Continue" takes you to the external link.
github-actions[bot] commented 1 year ago

Netlify Deployments:
www: https://ocw-hugo-themes-www-pr-1269--ocw-next.netlify.app/
Course v2: https://ocw-hugo-themes-course-v2-pr-1269--ocw-next.netlify.app/

pdpinch commented 1 year ago

Thanks for the screen recording. I shared it with the OCW team. The design is great, BUT the video revealed an edge case that we didn't consider.

One of those links goes to https://www.ocw-openmatters.org, which the OCW team considers to be an internal site. Is there a reasonable way to add it to the list of sites considered "internal" ?

HussainTaj-arbisoft commented 1 year ago

Is there a reasonable way to add it to the list of sites considered "internal"?

We can create a whitelist of sorts and treat those links as internal links. The current code can easily be extended to include this functionality.

We will need the complete list of domains and patterns to exclude. It will help us decide what string comparison approach to use.

I'll share the complete list of external URLs currently being used in production.

HussainTaj-arbisoft commented 1 year ago

A complete list of external URLs is shared here: https://github.com/mitodl/hq/issues/1812#issuecomment-1778996149

pdpinch commented 1 year ago

@HussainTaj-arbisoft what remains to be done here?

HussainTaj-arbisoft commented 1 year ago

@HussainTaj-arbisoft what remains to be done here?

@pdpinch, the exclusions remain to be implemented. I had a suggestion about it and I'm waiting for feedback on https://github.com/mitodl/hq/issues/1812#issuecomment-1788850100

HussainTaj-arbisoft commented 11 months ago

@gumaerc, I've updated this PR according to our new requirements. It is ready to be looked at again.

There are now three types of behaviors links will show.

Menu items can now have a parameter called includeLicenseWarning. You may view an example here.

Testing instructions are the same as before. For reference and testing, you may take this course content. Alternatively, this could also be tested as a part of https://github.com/mitodl/ocw-studio/pull/2034.

HussainTaj-arbisoft commented 11 months ago

@gumaerc, this PR is also on hold for now. Since it incorporates changes of https://github.com/mitodl/ocw-studio/pull/2034, it won't require a review unless we move ahead with that change.

HussainTaj-arbisoft commented 8 months ago

Superseded by https://github.com/mitodl/ocw-hugo-themes/pull/1364