Subscribe to newsletter form response #245 #253
Conversation
manuel-vara
requested review from
Lakshmishri,
cogniSyb and
markovukiceviccn
|
Hello, I'm the AEM Code Sync Bot and I will run some test suites that validate the page speed.
|
|
|
|
|
|
|
cogniSyb
changed the title
|
|
blocks/pardot-form/pardot-form.js
Outdated
| import { loadScript, sampleRUM } from '../../scripts/lib-franklin.js'; | ||
| import { getTextLabel } from '../../scripts/common.js'; | ||
|
|
||
| const thankyouMessage = `<p class='pardot-forms-thanks-title'>${getTextLabel('Successful submission title')}</p> |
There was a problem hiding this comment.
As 'success' is used elsewhere, please rename thankyouMessage to successMessage
blocks/pardot-form/pardot-form.js
Outdated
| sampleRUM('form:submit'); | ||
| const thankyouDiv = document.createElement('div'); | ||
| thankyouDiv.innerHTML = thankyouMessage; | ||
| const form = document.querySelector('form'); |
There was a problem hiding this comment.
We could have multiple forms on the page, so this should be made more specific
blocks/pardot-form/pardot-form.js
Outdated
| import { loadScript, sampleRUM } from '../../scripts/lib-franklin.js'; | ||
| import { getTextLabel } from '../../scripts/common.js'; | ||
|
|
||
| const thankyouMessage = `<p class='pardot-forms-thanks-title'>${getTextLabel('Successful submission title')}</p> |
There was a problem hiding this comment.
Rename class pardot-forms-thanks-title to pardot-form__title pardot-form__title--success
blocks/pardot-form/pardot-form.js
Outdated
| import { getTextLabel } from '../../scripts/common.js'; | ||
|
|
||
| const thankyouMessage = `<p class='pardot-forms-thanks-title'>${getTextLabel('Successful submission title')}</p> | ||
| <p class='pardot-forms-thanks-text'>${getTextLabel('Successful submission text')}</p> |
There was a problem hiding this comment.
Rename class pardot-forms-thanks-text to pardot-form__text pardot-form__text--success
blocks/pardot-form/pardot-form.js
Outdated
| `; | ||
|
|
||
| const errorMessage = `<p class='pardot-forms-error-title'>${getTextLabel('Error submission title')}</p> | ||
| <p class='pardot-forms-error-text'>${getTextLabel('Error submission text')}</p> |
There was a problem hiding this comment.
Change this class and the one of the title in the same way as above
|
|
||
| const blockName = 'v2-newsletter'; | ||
|
|
||
| const thankyouMessage = `<p class='pardot-forms-thanks-title'>${getTextLabel('Successful submission title')}</p> |
There was a problem hiding this comment.
In order to end up with the design:
const successTitle = `${getTextLabel('Successful submission title')}`;
const successText = `${getTextLabel('Successful submission text')}`;
async function submissionSuccess() {
sampleRUM('form:submit');
const form = document.querySelector('form');
const title = document.querySelector(`.${blockName}__title`);
const message = document.createElement('p');
message.textContent = successText;
title.textContent = successTitle;
form.replaceWith(message);
}
|
|
There was a problem hiding this comment.
In v2-newsletter.css add the following declaration
.v2-newsletter__form-container p {
margin: 0;
max-width: var(--text-block-max-width);
text-align: center;
}At the level of .v2-newsletter__form-container .button and remove this entire redundant declaration:
.v2-newsletter__form-container .button {
text-transform: uppercase;
}Thanks.
Should we add new css classes to handle validation/response errors? It seems subscribe component has some designs for it.
|
|
||
| const blockName = 'v2-newsletter'; | ||
|
|
||
| //* init response handling * | ||
| const successTitle = `${getTextLabel('Success newsletter title')}`; |
There was a problem hiding this comment.
My bad over here, this should be const successTitle = getTextLabel('Successful submission title'); instead. Same pattern for the line below
There was a problem hiding this comment.
That's fine, I believe. I've just created 2 new placeholder entries with those keys to align subscribe to newsletter messages provided in Figma designs. In that way we can show different messages depending on the custom block where v2-forms block is used, giving us an extra level of response feedback.
| } | ||
| //* end response handling * | ||
|
|
||
| // eslint-disable-next-line func-names |
There was a problem hiding this comment.
Remove comment, we can solve it like this:
window.logResult = function logResult(json) {
if (json.result === 'success') {
submissionSuccess();
} else if (json.result === 'error') {
submissionFailure();
}
};
|
|
|
|
|
Please always provide the GitHub issue(s) your PR is for, as well as test URLs where your change can be observed (before and after):
Fix #245
Test URLs:
New Block
Fully authorable form block with form definition in spreadsheet format.
Updated Blocks
Includes the update of v2-forms & v2-newsletter to manage
Form Handlerresponses.New custom forms would need to include v2-forms + specific handling response.