Feat/contact us page #260

gweiying · 2020-11-24T18:15:26Z

Addresses #217. PR is continued in #266.

This PR is still a work in progress, but seeking feedback for the stuff here so far. The remaining tasks are more modular/ cleanup-focused, and is unlikely to affect functionality significantly. Please have a look and give your feedback, especially if it's not covered in the scope of the tasks mentioned below or it's a bug I've not discovered:

Remaining tasks:

~~Initialize contact-us.md if file does not exist (P1)~~ Hide/ Show Contact Us card on Workspace depending on whether contact-us.md exists (P1)
Validation and Error Checking (P1)
~~- [ ] Add CSP checking on Contact Description field as html elements are accepted (P1)~~
Clean up implementation/ modification of POST update for Settings on backend (P1) (see link)
Auto format phone numbers with spacing, prefixes when a user enters the number (how to support both toll-free and local numbers?) (P1)
Remove empty object from the operatingHours array ~~, either through validation or~~ through filtering (P2)
Change color of Agency Name on Edit Contact Us preview to use Site theme color (P2)
Clean up PropTypes and setting default PropType values (P2)
~~- [ ] Contact Us image ? (see link)~~
- [ ] Data sanitiser should return indicator for whether values are removed e.g. Customs has more Address fields than we support, so we automatically truncate it to the first 3 fields. Warning should be returned when fields are removed. (to be discussed)
Clean up this.state.displaySections, intended structure: { contact: [], location: [], sectionsDisplay: {} } as this should simplify displayHandler, createHandler, deleteHandler (P3)
Code formatting (P3)

Additional tasks (updated 11/26/2020):

Additional tasks (updated 12/1/2020):

Fix title overflow on card
Add note to Map Url about automated link generation
Make links open in new page (consistent behavior w template)
Red border on error cards
Keep delete confirmation modal
Validation title: Length 1 and above

For FUTURE PR (updated 12/1/2020):

Validation on mount
Providing overview of removed data
~~Fix memory leaks with API call on Workspace~~
Contact Us script python
Filter out contact-us page from /pages in Workspace
Option to delete all location/ contact cards at once
Allow deletion of contact us page

Considerations to discuss with Lisa/ designers - Updated summary of discussion on 12/1 linked here

Should links on preview pages work?
View Map url generation (how to tell users that we will generate the url automatically)?
How to deal with already erroneous data on mount? (sanitise, validation) How to feedback this to users?
How to highlight which cards have errors?
Feedback field position - when you type how to correlate positions?
Limit on # of contact cards/ location cards
Contact us page image?
Do we need the Delete confirmation modal if all we are deleting is just one Contact/ one Location card? Tradeoff between extra click + flow-interruption vs having to retype card if accidentally deleted.
Option to delete all Location/ Card cards at once? Otherwise a user has to individually open each card to delete it.
Allow deletion of page if they don't want a contact-us.md page?

alexanderleegs

Left a few comments. In addition, there were some other things we could possibly consider modifying:

Editing a field automatically shifts the editor view back to the start of a section (e.g. editing a location will bring you to the very first location), which is problematic for long sections - the viewer should only snap back to the item being edited
We might want to provide a way to add and delete sections - there's no way to remove or add a new section entirely currently
We could implement a check for any edited data and give a warning if the user is navigating away with unsaved changes, similar to editHomepage
One other thing is that on a site with no contact-us to begin with, there's no way to add a contact-us page. We could possible consider giving the option to create a new contact-us page, which gives default values? This could also be used in the case that the contact-us page is empty. Am open to suggestions on this

src/components/FolderCard.jsx

src/utils/validators.js

kwajiehao

Left a couple of comments - other comments to be discussed offline

kwajiehao · 2020-11-25T20:17:50Z

src/layouts/Workspace.jsx

-            setCollections(collectionsResp.data?.collections)
+      const fetchData = async () => {
+        try { 
+          await axios.get(`${BACKEND_URL}/sites/${siteName}/pages/contact-us.md`);


For this part, you might want to rebase to the latest staging branch. To avoid memory leaks with this API call (refer to #242), we use a local variable _isMounted and only update state when _isMounted === true, so in this case it would be

if (_isMounted) setContactUsCard(true)

kwajiehao · 2020-11-25T20:55:57Z

src/templates/contact-us/ContactsSection.jsx

+          default: { // others  
+            return (
+              /* TODO: CSP validation should be done on html elements before rendering */
+              <div dangerouslySetInnerHTML={{__html: d[key]}} key={i}/>


I forgot what the use case was here - why is this not also a  like the others? Reference from the customs website for this portion:

For queries and appeals pertaining to a Notice of Customs Offence by Singapore Customs Special Investigation Branch

We support html tags here, such for A-star NRP raw file:

- title: Feedback content: - other: We would love to hear what you have to say! Fill in a feedback form <a href="https://form.gov.sg/#!/5fab84f4ec6c950011f3e8ba">here</a>!

I forgot what the conclusion was here - the problem is that in practice the html tags are wrapped inside the  tag, but we're unable to dangerously set inner html for p tags right?

kwajiehao · 2020-11-26T04:05:42Z

Left a few comments. In addition, there were some other things we could possibly consider modifying:

Editing a field automatically shifts the editor view back to the start of a section (e.g. editing a location will bring you to the very first location), which is problematic for long sections - the viewer should only snap back to the item being edited

We might want to provide a way to add and delete sections - there's no way to remove or add a new section entirely currently

We could implement a check for any edited data and give a warning if the user is navigating away with unsaved changes, similar to editHomepage

One other thing is that on a site with no contact-us to begin with, there's no way to add a contact-us page. We could possible consider giving the option to create a new contact-us page, which gives default values? This could also be used in the case that the contact-us page is empty. Am open to suggestions on this

@alexanderleegs RE: the last point on no Contact Us page, Alexis and I had discussed this separately and since most repositories have a Contact Us page, we'll first go off the assumption that repos have a contact us page, and if they don't, then just don't display the Contact Us card for now.

gweiying · 2020-11-26T17:20:55Z

^ Deleted @kwajiehao's comment accidentally, comment was about multi-line form field validation vs single-line form field validation (can't find option to undo delete)

kwajiehao

Left comments

kwajiehao · 2020-11-30T05:35:16Z

src/utils/validators.js

+const URL_REGEX_PART_2 = '.com/)([a-zA-Z0-9_-]+(/)?)+$';
+const PHONE_REGEX = '^\\+65(6|8|9)[0-9]{7}$'
+const TOLLFREE_PHONE_REGEX = '^1800[0-9]{7}$'
+const EMAIL_REGEX = '^[a-z0-9-_\.]+@[a-z0-9-]+\.[a-z]{2,4}$';


Can we consider using a more established email regex (like https://emailregex.com/)

good idea, will change this

kwajiehao · 2020-11-30T05:41:36Z

src/templates/contact-us/ContactsSection.jsx

+          default: { // others  
+            return (
+              /* TODO: CSP validation should be done on html elements before rendering */
+              <div dangerouslySetInnerHTML={{__html: d[key]}} key={i}/>


I forgot what the conclusion was here - the problem is that in practice the html tags are wrapped inside the  tag, but we're unable to dangerously set inner html for p tags right?

kwajiehao · 2020-11-30T05:42:39Z

src/layouts/EditContactUs.jsx

+          break;
+        }
+        case 'location': { 
+          const { locations } = state.frontMatter;


Unused line of code

this has been fixed in the latest branch of code https://github.com/isomerpages/isomercms-frontend/tree/feat/contact-us-page-2

kwajiehao · 2020-11-30T05:46:43Z

src/utils/validators.js

+      if (value && !alphanumericRegexTest.test(value)) {
+        errorMessage += `Field should only contain alphanumeric characters. `
+      }
+      if (value && !value.length < LOCATION_OPERATING_HOURS_MIN_LENGTH) {


There is a bug here, it should be value.length < LOCATION_OPERATING_HOURS_MIN_LENGTH instead of !value.length < LOCATION_OPERATING_HOURS_MIN_LENGTH

this has been fixed in the latest branch of code https://github.com/isomerpages/isomercms-frontend/tree/feat/contact-us-page-2

kwajiehao · 2020-11-30T05:46:54Z

src/utils/validators.js

+      if (value && !value.length < LOCATION_OPERATING_HOURS_MIN_LENGTH) {
+        errorMessage += `Field should be longer than ${LOCATION_OPERATING_HOURS_MIN_LENGTH} characters.`
+      }
+      if (value && !value.length > LOCATION_OPERATING_HOURS_MAX_LENGTH) {


Same bug as above

this has been fixed in the latest branch of code https://github.com/isomerpages/isomercms-frontend/tree/feat/contact-us-page-2

kwajiehao

lgtm, remaining changes will be reviewed in #266

alexanderleegs

lgtm

… Feedback Sections)

…tact-us template page, renames old Breadcrumb as PageHeader and updates Breadcrumb references in LeftNavPage and SimplePage as well as isomer-template.scss

… isHomepage and isCollection to a single page type string as there are 4 different page types to consider now, updates references to isHomepage and isCollection in Workspace, FolderCard and Resources, adds contact-us route to App

…lly intended for reuse in contact contents; updates NewSectionCreator to use Dropdown component

…used as a series of fields without each having a title such as the address fields in Contact Us

…ctCards, Section, Fields)

…act-us.md file exists

…epage templates structure

* fix: suppresses dropdown for Contact Us Folder Card dropdown options * fix: removes hardcoded sectionId, passing sectionId as argument * fix: updates displaySections to use locations, contacts as key values, simplifies indexing updates for OnDragEnd, createHandler, deleteHandler, displayHandler * fix: adds check if frontMatter.contacts or frontMatter.locations array is empty, if empty the key-value pair is removed and not saved * fix: updates templates to use forwarded react references * fix: updates scrollRefs to use React.createRef() and forward references to templates, also updates scroll behavior for onFieldChange, onDragEnd, deleteHandler, createHandler and displayHandler * fix: fixes error in operatingHours validation, adds case for validating map links * fix: clean up comments, whitespace * fix: checks whether content has been edited and shows navigate away warning accordingly * fix: uses deep cloning to sanitise frontMatter * fix: sets background color for contactUs edit preview * fix: buffs up email validation * fix: add packages for DOMPurify * fix: uses DOMPurify to prevent XSS scripts from rendering on Contacts descriptions * fix: fixes paragraph stylistic formatting on Contacts description * fix: fixes form field width for operating hours to not resize with error length * fix: resolves stylistic differences in links and span text * fix: adds parent style around links in ContactSection Templates * fix: cleans up scrollIntoView * fix: adds key to dropdown options to resolve unique id warning * install: adds packages for react-input-mask * feat: creates InputMaskFormField using react-input-mask for FormFields that can use masked input styling, for example dates, phone numbers and so on * feat: adds state hook to Contact Fields to support masked input styling for the Phone Number field, uses Dropdown to set state * fix: simplifies validation for Contact Fields phone number as masked input provides user cues for length of phone number already * fix: rectifies relative imports * fix: cleans up warning errors in validation * fix: fix initialized state in state hook * fix: filters dropdown before mapping to remove arrow function warning * fix: sets font size for error and info messages since span has been set by default to inherit parent font-size * fix: uses info message style for notes * fix: removes extraneous terms from automated url generation and sets View Map link to open in a new page * fix: adds Delete modal for each Location and Contact * feat: add styling for error cards * feat: uses red border to highlight sections and cards that contain errors, border is only highlighted when section/card is minimized * fix: fix title overflow bug * fix: removes default value if user does not select phone number field * fix: clean up dataSanitisers and add initial values for maps_links and titles * fix: removes min length for title * fix: sets min length of titles to 1 * fix: change Add button to singular form instead of plural as in Contact and Location not contacts and locations

gweiying mentioned this pull request Nov 24, 2020

fix: update config settings and footer settings separately isomerpages/isomercms-backend#81

Merged

gweiying requested review from kwajiehao and alexanderleegs and removed request for kwajiehao November 25, 2020 09:14

alexanderleegs requested changes Nov 26, 2020

View reviewed changes

src/components/FolderCard.jsx Outdated Show resolved Hide resolved

src/utils/validators.js Outdated Show resolved Hide resolved

kwajiehao reviewed Nov 26, 2020

View reviewed changes

isomerpages deleted a comment from kwajiehao Nov 26, 2020

kwajiehao reviewed Nov 30, 2020

View reviewed changes

gweiying linked an issue Dec 1, 2020 that may be closed by this pull request

EditContactUsPage layout #217

Closed

kwajiehao approved these changes Dec 3, 2020

View reviewed changes

alexanderleegs approved these changes Dec 4, 2020

View reviewed changes

gweiying added 16 commits December 4, 2020 08:39

feat: add template components for contact-us page (Contact, Location,…

bc59725

… Feedback Sections)

feat: refactors Breadcrumb into a separate component for reuse in con…

542ec2c

…tact-us template page, renames old Breadcrumb as PageHeader and updates Breadcrumb references in LeftNavPage and SimplePage as well as isomer-template.scss

feat: refactors Dropdown into a separate component for reuse, origina…

908ecbd

…lly intended for reuse in contact contents; updates NewSectionCreator to use Dropdown component

fix: makes FormField title non-compulsory so that component can be re…

a6cae1e

…used as a series of fields without each having a title such as the address fields in Contact Us

feat: add editor components for Contact Us page (LocationCards, Conta…

ffc4e2e

…ctCards, Section, Fields)

feat: add util functions for contact us

10c791f

feat: add contact us editor page

cffa009

feat: add data sanitisers for contact us

5a83d81

fix: show/hides Contact Us button from Workspace depending on if cont…

1124305

…act-us.md file exists

fix: loop through contacts, locations arrays only if arrays are defined

4203d4b

fix: fix typo in dataSanitisers equality check

1b690c1

fix: updates AgencyName to use theme color

3d8aa31

fix: fix class warning error, changes class to className

c850a99

fix: removes empty operating hours objects upon saving

11940e3

fix: changes contact-us templates structure to be consistent with hom…

e875e28

…epage templates structure

gweiying and others added 6 commits December 4, 2020 08:53

fix: fixes class warning errors, replaces class with className

a651e19

feat: adds validation and error checking to edit contact us page

433ed94

fix: fixes link size on sidebar

b589356

fix: line height for card header overflow

41bbfc7

fix: fix page type on foldercard

9548a83

gweiying force-pushed the feat/contact-us-page branch from c432942 to 9548a83 Compare December 4, 2020 08:24

gweiying merged commit aefeae3 into staging Dec 4, 2020

gweiying deleted the feat/contact-us-page branch December 4, 2020 09:30

kwajiehao mentioned this pull request Dec 9, 2020

Merge to Prod - 23 Dec 2020 #287

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/contact us page #260

Feat/contact us page #260

gweiying commented Nov 24, 2020 •

edited

Loading

alexanderleegs left a comment

kwajiehao left a comment

kwajiehao Nov 25, 2020

kwajiehao Nov 25, 2020

gweiying Nov 26, 2020 •

edited

Loading

kwajiehao Nov 30, 2020

kwajiehao commented Nov 26, 2020

gweiying commented Nov 26, 2020

kwajiehao left a comment

kwajiehao Nov 30, 2020

gweiying Nov 30, 2020

kwajiehao Nov 30, 2020

kwajiehao Nov 30, 2020

gweiying Nov 30, 2020

kwajiehao Nov 30, 2020

gweiying Nov 30, 2020

kwajiehao Nov 30, 2020

gweiying Nov 30, 2020

kwajiehao left a comment

alexanderleegs left a comment

Feat/contact us page #260

Feat/contact us page #260

Conversation

gweiying commented Nov 24, 2020 • edited Loading

Remaining tasks:

Additional tasks (updated 11/26/2020):

Additional tasks (updated 12/1/2020):

For FUTURE PR (updated 12/1/2020):

Considerations to discuss with Lisa/ designers - Updated summary of discussion on 12/1 linked here

alexanderleegs left a comment

Choose a reason for hiding this comment

kwajiehao left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gweiying Nov 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwajiehao commented Nov 26, 2020

gweiying commented Nov 26, 2020

kwajiehao left a 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

kwajiehao left a comment

Choose a reason for hiding this comment

alexanderleegs left a comment

Choose a reason for hiding this comment

gweiying commented Nov 24, 2020 •

edited

Loading

gweiying Nov 26, 2020 •

edited

Loading