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.

Sign up for GitHub

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

Add new Modal Dialogue component #1668

Closed
wants to merge 5 commits into from
Closed

Add new Modal Dialogue component #1668

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7be5dcf
Fix increased margin above/below component preview
colinrotherham Nov 27, 2019
aae7ff1
Add support for Nunjucks macro call blocks
colinrotherham Nov 27, 2019
956d5ae
Add fixed-height “modal” preview template
colinrotherham Nov 27, 2019
e346314
Fix full-width layout in IE8
colinrotherham Nov 28, 2019
aba92ba
Add modal dialogue component
colinrotherham Nov 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 26 additions & 7 deletions app/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,6 @@ module.exports = (options) => {
const componentName = req.params.component
const requestedExampleName = req.params.example || 'default'

const previewLayout = res.locals.componentData.previewLayout || 'layout'

const exampleConfig = res.locals.componentData.examples.find(
example => example.name.replace(/ /g, '-') === requestedExampleName
)
Expand All @@ -164,16 +162,37 @@ module.exports = (options) => {

// Construct and evaluate the component with the data for this example
const macroName = helperFunctions.componentNameToMacroName(componentName)
const macroParameters = JSON.stringify(exampleConfig.data, null, '\t')
const macroContext = { data: exampleConfig.data, caller: exampleConfig.caller }

// Build macro view
let macroString = `{% from '${componentName}/macro.njk' import ${macroName} %}`

// Macro is a caller
if (typeof exampleConfig.caller === 'object') {
macroString += `
{% call ${macroName}(data) %}
{{ caller.html | safe if caller.html else caller.text }}
{% endcall %}
`
} else {
macroString += `{{ ${macroName}(data) }}`
}

res.locals.componentView = env.renderString(
`{% from '${componentName}/macro.njk' import ${macroName} %}
{{ ${macroName}(${macroParameters}) }}`
)
res.locals.componentView = env.renderString(macroString, macroContext)

// Optional preview layout
let previewLayout = res.locals.componentData.previewLayout
let bodyClasses = ''

// Customise iframe preview
if (req.query.iframe) {
bodyClasses = 'app-iframe-in-component-preview'

if (previewLayout) {
bodyClasses += ` app-iframe-in-component-preview--${previewLayout}`
}

previewLayout = 'component-iframe'
}

res.render('component-preview', { bodyClasses, previewLayout })
Expand Down
29 changes: 26 additions & 3 deletions app/assets/scss/partials/_app.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@

// Injected into the preview page, that is then put in an iframe
.app-iframe-in-component-preview {
margin: 15px 0;
margin: 5px;
}

.app-iframe-in-component-preview--full-width {
margin-right: 0;
margin-left: 0;
}

// Removes the breakpoint sass-mq debug display
Expand All @@ -22,9 +27,17 @@

.app-component-preview {
position: relative;
@include govuk-responsive-margin(5, "top");
@include govuk-responsive-margin(5, "bottom");
}

// Full width for header/footer etc
.app-component-preview--full-width,
.app-component-preview--modal {
width: 100%;
hannalaakso marked this conversation as resolved.
Show resolved Hide resolved
margin-top: -15px;
margin-bottom: 15px;
max-width: none;
margin-right: 0;
margin-left: 0;
}

.app-component-preview__iframe {
Expand All @@ -46,6 +59,7 @@
content: " ";
display: table;
width: 100%;
height: 100%;
hannalaakso marked this conversation as resolved.
Show resolved Hide resolved
clear: both;
box-shadow: 0 0 0 5px $app-preview-border-colour;

Expand All @@ -54,3 +68,12 @@
}
}
}

// Allow fixed height for "position: fixed" modal
.app-iframe-in-component-preview--modal {
height: 400px;

@include govuk-media-query ($from: tablet) {
height: 500px;
}
hannalaakso marked this conversation as resolved.
Show resolved Hide resolved
}
21 changes: 21 additions & 0 deletions app/views/layouts/component-iframe.njk
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{% extends "_generic.njk" %}

{% set htmlClasses = "app-no-canvas-background" %}

{% block pageTitle %}GOV.UK Frontend{% endblock %}

{# Turn the header and footer off #}
{% block header %}
{% include "../partials/banner.njk" %}
{% endblock %}
{% block footer %}{% endblock %}

{% block main %}
{% block beforeContent %}{% endblock %}
{% block content %}{% endblock %}
{% endblock %}

{% block bodyEnd %}
{{ super() }}
{% block scripts %}{% endblock %}
{% endblock %}
1 change: 1 addition & 0 deletions app/views/layouts/component-preview.njk
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{% set previewLayout = previewLayout | default('layout') %}
{% extends previewLayout + ".njk" %}

{% set bodyClasses %}
Expand Down
7 changes: 7 additions & 0 deletions app/views/layouts/modal.njk
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{% extends "layout.njk" %}

{% block main %}
<main class="govuk-main-wrapper {{ mainClasses }}" id="main-content" role="main">
{% block content %}{% endblock %}
</main>
{% endblock %}
2 changes: 1 addition & 1 deletion app/views/macros/showExamples.njk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
</p>
{% endif %}
</div>
<div class="app-component-preview">
<div class="govuk-width-container app-component-preview {%- if data.previewLayout %} app-component-preview--{{ data.previewLayout }}{% endif %}">
<iframe
class="js-component-preview app-component-preview__iframe"
src="{{ path + iframePathQuery }}"
Expand Down
6 changes: 6 additions & 0 deletions src/govuk/all.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import CharacterCount from './components/character-count/character-count'
import Checkboxes from './components/checkboxes/checkboxes'
import ErrorSummary from './components/error-summary/error-summary'
import Header from './components/header/header'
import ModalDialogue from './components/modal-dialogue/modal-dialogue'
import Radios from './components/radios/radios'
import Tabs from './components/tabs/tabs'

Expand Down Expand Up @@ -50,6 +51,11 @@ function initAll (options) {
var $toggleButton = scope.querySelector('[data-module="govuk-header"]')
new Header($toggleButton).init()

var $modals = scope.querySelectorAll('[data-module="govuk-modal-dialogue"]')
nodeListForEach($modals, function ($modal) {
new ModalDialogue($modal).init()
})

var $radios = scope.querySelectorAll('[data-module="govuk-radios"]')
nodeListForEach($radios, function ($radio) {
new Radios($radio).init()
Expand Down
1 change: 1 addition & 0 deletions src/govuk/components/_all.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
@import "input/input";
@import "inset-text/inset-text";
@import "label/label";
@import "modal-dialogue/modal-dialogue";
@import "panel/panel";
@import "phase-banner/phase-banner";
@import "tabs/tabs";
Expand Down
Empty file added src/govuk/components/modal-dialogue/README.md
View file
Open in desktop
Empty file.
161 changes: 161 additions & 0 deletions src/govuk/components/modal-dialogue/_modal-dialogue.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
@import "../../settings/all";
@import "../../tools/all";
@import "../../helpers/all";

@include govuk-exports("govuk/component/modal-dialogue") {
$govuk-dialogue-width: 640px;
Copy link
Member

Choose a reason for hiding this comment

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

We probably want to use !default here so that the value can overridden.


.govuk-modal-dialogue,
.govuk-modal-dialogue__backdrop {
position: fixed;
z-index: 0;
Copy link

@nogginbox nogginbox Jul 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Increase to 1000 so that this will appear above all other elements already in the site.

I had to do this when I've used this code as several elements floated on top of the backdrop.

Suggested change
z-index: 0;
z-index: 1000;

top: 0;
left: 0;
width: 100%;
height: 100%;
}

// Hide dialogue when closed
.govuk-modal-dialogue {
display: none;
}

// Show dialogue when opened
.govuk-modal-dialogue--open {
display: block;
}

// Wrapper to handle overflow scrolling
.govuk-modal-dialogue__wrapper {
box-sizing: border-box;
display: flex;
height: 100%;
@include govuk-responsive-padding(7, "top");
@include govuk-responsive-padding(7, "bottom");
overflow-y: auto;
align-items: flex-start; // sass-lint:disable no-duplicate-properties
align-items: safe center;
}

// HTML5 dialogue component
.govuk-modal-dialogue__box {
box-sizing: border-box;
display: block;
position: relative;
z-index: 1;
Copy link
Member

Choose a reason for hiding this comment

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

We might want to increase this, just in the (non-ideal) case that the page has other competing elements. I notice that the GOV.UK Publishing equivalent sets this to 1000.

Copy link

Choose a reason for hiding this comment

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

I've made this 1001 so it is above the backdrop which also needs the z-index increasing.

Suggested change
z-index: 1;
z-index: 1001;

width: 90%;
margin: auto;
padding: 0;
overflow-y: auto;
Copy link
Member

@hannalaakso hannalaakso Dec 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

This might be okay as it is but just for reference, I notice that the GOV.UK Publishing example currently sets these to:

overflow-x: hidden;
overflow-y: scroll;

I don't think we currently have an example with page content in the main area so we need to add one (see #1668 (comment)) in order to check that the main page content is not scrollable and the modal is.

border: $govuk-focus-width solid govuk-colour("black");
background: govuk-colour("white");

// Add focus outline to dialogue
&:focus {
outline: $govuk-focus-width solid $govuk-focus-colour;
}

// Hide browser backdrop
&::backdrop {
display: none;
}
}

// Header with close button
.govuk-modal-dialogue__header {
@include govuk-clearfix;
@include govuk-responsive-margin(5, "bottom");
padding-bottom: $govuk-focus-width;
color: govuk-colour("white");
background-color: govuk-colour("black");
text-align: right;
}

// Inner content
.govuk-modal-dialogue__content {
@include govuk-font($size: 16);
Copy link

Choose a reason for hiding this comment

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

I was able to safely remove this font definition as I wanted the site's standard fonts to be used. We're a non gov body so can't use the standard GDS font and have overrides in place to not use them. This line being included broke our GDS font overrides.

Suggested change
@include govuk-font($size: 16);

@include govuk-responsive-padding(6);
padding-top: 0;
}

.govuk-modal-dialogue__description {
@include govuk-responsive-margin(4, "bottom");
}

// Remove bottom margins
.govuk-modal-dialogue__description:last-child,
.govuk-modal-dialogue__description > :last-child,
.govuk-modal-dialogue__content > :last-child {
margin-bottom: 0;
}

// Custom backdrop
.govuk-modal-dialogue__backdrop {
opacity: .8;
background: govuk-colour("black");
pointer-events: none;
touch-action: none;
}
Copy link
Member

Choose a reason for hiding this comment

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

I think this block should live below https://github.com/alphagov/govuk-frontend/pull/1668/files#diff-40be96fa6566e06e9d9d9ab4d1d3d831R8 to in order to keep the backdrop styles together.


// Crown icon
.govuk-modal-dialogue__crown {
display: block;
margin: 6px 0 0 6px;
@include govuk-responsive-margin(5, "left");
float: left;
}

// Heading
.govuk-modal-dialogue__heading:last-child {
margin-bottom: 0;
}

// Close button
.govuk-modal-dialogue__close {
$font-size: 36px;
$line-height: 1;

display: block;
width: auto;
min-width: 44px;
margin: 0;
padding: 2px 5px;
float: right;
color: govuk-colour("white");
background-color: govuk-colour("black");
box-shadow: none !important;
font-size: $font-size;
@if $govuk-typography-use-rem {
font-size: govuk-px-to-rem($font-size);
}
@include govuk-typography-weight-bold;
line-height: $line-height;

&:hover {
color: govuk-colour("black");
background-color: govuk-colour("yellow");
}

&:active {
top: 0;
}
}

// New dialogue width, inline button + link
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure why these styles are set separately from the other ones in this file. Should this be a modifier class?

@include govuk-media-query($from: tablet) {
Copy link
Member

Choose a reason for hiding this comment

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

Is the mobile/tablet experience meant to be similar to the GOV.UK Publishing modal?

Screen Shot 2019-12-10 at 16 52 02

This might be something for @Ash-Wilson to consider.

.govuk-modal-dialogue__content {
padding-top: 0;
}

.govuk-modal-dialogue__box {
width: percentage($govuk-dialogue-width / map-get($govuk-breakpoints, desktop));
}
}

// Fixed width
@include govuk-media-query($from: desktop) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure why these styles are set separately from the other ones in this file. Should this be a modifier class?

.govuk-modal-dialogue__box {
width: $govuk-dialogue-width;
}
}
}
3 changes: 3 additions & 0 deletions src/govuk/components/modal-dialogue/macro.njk
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{% macro govukModalDialogue(params) %}
{%- include "./template.njk" -%}
{% endmacro %}
Loading