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.

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

Page feedback tool: Revamp #1916

Closed

Conversation

EricDunsworth
Copy link
Member

@EricDunsworth EricDunsworth commented Dec 10, 2021

Progress checklist:
#1916 (comment)

General:

  • Rename "page success widget" to "page feedback tool"
  • Turn it into a site component

Jekyll:

  • Change the tool into an include
  • Use sub-includes for parameter/string variable declarations
  • Add a pageFeedback variable to enable the tool in page templates
  • Add pageFeedback sub-variables to optionally override the default values of the tool's form parameters:
    • institutionopt
    • themeopt
    • sectionopt
    • pageTitle (true re-uses the current page's title, without " - Canada.ca")
    • language (true re-uses the current page's language)
    • submissionPage (true re-uses the current page's URL)
  • Update the page details footer include to accommodate the tool:
    • Declare col-modified = "col-xs-12" only once at the beginning of variable assignments
    • Support pageFeedback and prioritize it over report a problem
    • Make pageFeedback take priority over noReportProblem (i.e. setting former to true and latter to false will show the page feedback tool)

SCSS:

  • Add new classes (tied to the gc-pft class):
    • nojs-col-sm-12
    • nojs-pr-sm-0
    • nojs-text-left
  • Increase the share widget's top margin to make it look vertically-centered when used alongside the tool in small/medium view and over
    • Depends on the has() pseudo-class

HTML:

  • Rename the gc-pg-hlpfl id/class prefix to gc-pft
  • Rename the gc-pg-hlpfl-btn class to gc-pft-btns ("s" suffix)
  • Use a fieldset for "Did you find what you were looking for?" and its yes/no buttons
  • Change "Did you find what you were looking for?" into a legend
  • Improve layout:
    • Remove heading classes
    • Use form classes (i.e. chkbxrdio-grp, field-name and form-group)
    • Increase spacing between groups of content
    • Move the details field's secondary information paragraph below the field
    • JS mode:
      • Center-align "Did you find what you were looking for?" in extra-small view
      • Vertically-center "Did you find what you were looking for?" and yes/no buttons in small view and over
    • Noscript/Basic HTML mode:
      • Position the yes button directly below "Did you find what you were looking for?" in all views
  • Use aria-live regions to announce transition messages to screen reader users:
    • "Tell us why below:" upon pressing the no button
    • "Thank you for your feedback." upon pressing the yes button or submitting the no button's questionnaire
  • Add aria-describedby attributes to:
    • Programmatically-associate the yes button to the "If not, tell us why below:" paragraph in noscript/basic HTML mode (to make screen readers announce the no button's instructions right after the yes button)
    • Programmatically-associate the secondary information paragraph to the more details field

Content:

  • Add a visually-hidden "Page feedback" H3 heading
  • Add a period to the end of "Thank you for your feedback"
  • Add "below" to the end of "If not, tell us why:" in noscript/basic HTML mode
  • Add a visually-hidden "Tell us why below:" paragraph in JS mode
  • Combine "Please provide more details" and "Maximum 300 characters" into the same label
  • Write all values in an "EN-FR" format, derived from labels

Demo pages:

  • Add a standard demo page (pageFeedback enabled)
  • Add a custom demo page (pageFeedback with sub-variables)

Provisional functionality page:

  • Portray the widget as stable in the feature availability table
  • Remove "page success widget" section/example

CC @HamzaAburaneh @LanaStewa @delisma

_data/components.json Outdated Show resolved Hide resolved
_data/components.json Outdated Show resolved Hide resolved
_data/components.json Outdated Show resolved Hide resolved
_data/components.json Outdated Show resolved Hide resolved
_data/components.json Outdated Show resolved Hide resolved
components/gc-page-success/gc-page-success-new-fr.html Outdated Show resolved Hide resolved
components/gc-page-success/gc-page-success-new-fr.html Outdated Show resolved Hide resolved
components/gc-page-success/index.json-ld Outdated Show resolved Hide resolved
components/gc-page-success/index.json-ld Outdated Show resolved Hide resolved
components/gc-page-success/index.json-ld Outdated Show resolved Hide resolved
@duboisp
Copy link
Member

duboisp commented Dec 18, 2021

FYI - This request will need a formal approval before we merge it. Additional information can be found with PP via the ref. number BSD-19805

\cc @GormFrank

@delisma
Copy link

delisma commented Dec 21, 2021

Yes. I've sent the ticket ref. #BSD-19805 earlier that morning with @HamzaAburaneh. Wasn't able to review the PR before sending the ticket. Did it later in the afternoon, to make sure that the ticket can refer to final commit of this PR.

@EricDunsworth EricDunsworth changed the title Page success widget: WIP updates Page feedback form: WIP updates Jan 28, 2022
@EricDunsworth EricDunsworth force-pushed the page-success-widget branch 26 times, most recently from ff2c18e to 753a18b Compare February 1, 2022 22:06
@EricDunsworth EricDunsworth temporarily deployed to github-ci November 16, 2022 20:02 Inactive
@EricDunsworth EricDunsworth temporarily deployed to github-ci November 16, 2022 20:17 Inactive
@EricDunsworth
Copy link
Member Author

Force-pushed some fixes:

  • Added a comment and include reference to _includes/variable-core.liquid before every code sample to restore missing strings:
    • Only the code samples need this. The widget depends on the i18nText-lang variable for language detection. It's built-into layout files, but Jekyll pages can't call layout variables. So I had to manually-reference that variable's include.
    • The page template's version of the widget doesn't need any of this since it can directly use the layout file's version of i18nText-lang.
  • Added an opuft_ prefix to sites/gc-page-feedback/includes/parameters.liquid's variables to restore missing form parameters
  • Added an "e" suffix to "personnalisé" in some of the French custom demo page's parameters
  • Added an error message to _includes/gc-page-feedback/gc-page-feedback.html in case the widget gets accidentally-activated in pages that can't actually use it
  • Updated modified dates

@EricDunsworth EricDunsworth temporarily deployed to github-ci November 29, 2022 17:45 Inactive
@EricDunsworth EricDunsworth changed the title Page feedback form: Revamp Page feedback tool: Revamp Nov 29, 2022
@EricDunsworth
Copy link
Member Author

@GormFrank gave me a heads up that this has been renamed again to "Page feedback tool (PFT)".

So I've just force-pushed the following changes:

  • Renamed these:
    • Code (SCSS classes/Jekyll variables):
      • opuft -> pft
    • Content:
      • Page feedback form -> Page feedback tool
      • Formulaire de rétroaction sur la page -> Outil de rétroaction sur la page
      • Feedback form for Canada.ca -> Page feedback tool for Canada.ca
      • Formulaire de rétroaction pour Canada.ca -> Outil de rétroaction sur la page pour Canada.ca
  • Updated commit message
  • Updated modified dates
  • Rebased
  • Kept file/folder names (gc-page-feedback) and Jekyll on/off variable name (pageFeedback) as-is

<td>v7.0</td>
<td></td>
<td>Stable (vX.X.X) (TODO)</td>
Copy link
Member Author

Choose a reason for hiding this comment

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

This will need to be filled-in at some point.

It'll represent the first GCWeb release that'll come with the page feedback tool.

<td>v7.0</td>
<td></td>
<td>Stable (vX.X.X) (TODO)</td>
Copy link
Member Author

Choose a reason for hiding this comment

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

Ditto.

@duboisp
Copy link
Member

duboisp commented Dec 5, 2022

Here a working example with the latest working example: https://servicecanada.github.io/wet-boew-demos/GCWeb-PR1916/sites/gc-page-feedback/gc-page-feedback-en.html

@EricDunsworth EricDunsworth marked this pull request as ready for review December 6, 2022 15:39
@EricDunsworth
Copy link
Member Author

FYI I've marked this as ready for review. Should be alright for PP to take this on now :).

Check out the checklist in #1916 (comment) to see what I believe is left.

PS:
Haven't discussed Jekyll logic with @delisma yet, but it's unlikely any changes will come about from that.

@EricDunsworth EricDunsworth temporarily deployed to github-ci December 6, 2022 16:05 Inactive
@EricDunsworth
Copy link
Member Author

Ugh just realized I forgot to rename the opuft classes to pft in my last commit message update. Just sorted it out in a force push.

Didn't change anything else and kept modified dates "as-is". So https://servicecanada.github.io/wet-boew-demos/GCWeb-PR1916/sites/gc-page-feedback/gc-page-feedback-en.html should still be accurate.

General:
* Rename "page success widget" to "page feedback tool"
* Turn it into a site component

Jekyll:
* Change the tool into an include
* Use sub-includes for parameter/string variable declarations
* Add a pageFeedback variable to enable the tool in page templates
* Add pageFeedback sub-variables to optionally override the default values of the tool's form parameters:
  * institutionopt
  * themeopt
  * sectionopt
  * pageTitle (true re-uses the current page's title, without " - Canada.ca")
  * language (true re-uses the current page's language)
  * submissionPage (true re-uses the current page's URL)
* Update the page details footer include to accommodate the tool:
  * Declare col-modified = "col-xs-12" only once at the beginning of variable assignments
  * Support pageFeedback and prioritize it over report a problem
  * Make pageFeedback take priority over noReportProblem (i.e. setting former to true and latter to false will show the page feedback tool)

SCSS:
* Add new classes (tied to the gc-pft class):
  * nojs-col-sm-12
  * nojs-pr-sm-0
  * nojs-text-left
* Increase the share widget's top margin to make it look vertically-centered when used alongside the tool in small/medium view and over
  * Depends on the has() pseudo-class

HTML:
* Rename the gc-pg-hlpfl id/class prefix to gc-pft
* Rename the gc-pg-hlpfl-btn class to gc-pft-btns ("s" suffix)
* Use a fieldset for "Did you find what you were looking for?" and its yes/no buttons
* Change "Did you find what you were looking for?" into a legend
* Improve layout:
  * Remove heading classes
  * Use form classes (i.e. chkbxrdio-grp, field-name and form-group)
  * Increase spacing between groups of content
  * Move the details field's secondary information paragraph below the field
  * JS mode:
    * Center-align "Did you find what you were looking for?" in extra-small view
    * Vertically-center "Did you find what you were looking for?" and yes/no buttons in small view and over
  * Noscript/Basic HTML mode:
    * Position the yes button directly below "Did you find what you were looking for?" in all views
* Use aria-live regions to announce transition messages to screen reader users:
  * "Tell us why below:" upon pressing the no button
  * "Thank you for your feedback." upon pressing the yes button or submitting the no button's questionnaire
* Add aria-describedby attributes to:
  * Programmatically-associate the yes button to the "If not, tell us why below:" paragraph in noscript/basic HTML mode (to make screen readers announce the no button's instructions right after the yes button)
  * Programmatically-associate the secondary information paragraph to the more details field

Content:
* Add a visually-hidden "Page feedback" H3 heading
* Add a period to the end of "Thank you for your feedback"
* Add "below" to the end of "If not, tell us why:" in noscript/basic HTML mode
* Add a visually-hidden "Tell us why below:" paragraph in JS mode
* Combine "Please provide more details" and "Maximum 300 characters" into the same label
* Write all values in an "EN-FR" format, derived from labels

Demo pages:
* Add a standard demo page (pageFeedback enabled)
* Add a custom demo page (pageFeedback with sub-variables)

Provisional functionality page:
* Portray the widget as stable in the feature availability table
* Remove "page success widget" section/example

Co-authored-by: David Elisma <david.elisma@tbs-sct.gc.ca>
@EricDunsworth EricDunsworth temporarily deployed to github-ci December 6, 2022 16:22 Inactive
@EricDunsworth
Copy link
Member Author

Guess I spoke too soon... looks like I forgot to rename the tool and the opuft classes in the provisional page. Just forced-pushed another fix for that and updated modified dates.

Sorry for these "extra" updates :(. I don't expect to be changing anything else from this point onwards without touching base with PP in advance.

@duboisp
Copy link
Member

duboisp commented Dec 12, 2022

@Garneauma can you rebuild the working example with the latest changes. Thanks

@Garneauma
Copy link
Contributor

Done. :)

https://servicecanada.github.io/wet-boew-demos/GCWeb-PR1916/sites/gc-page-feedback/gc-page-feedback-en.html

@GormFrank
Copy link
Collaborator

@duboisp I think this one can be closed now that PR #2098 has been merged, unless someone is opposed to it.

CC: @Garneauma @EricDunsworth

@EricDunsworth
Copy link
Member Author

EricDunsworth commented Aug 16, 2023

IMO it'd of been better to add "closes #1916" to #2098's OP prior to merging 😝.

But yeah I'm cool with closing-it out 👍.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants