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

wb-tables DataTable Utilities #2438

Merged
merged 11 commits into from
Nov 5, 2024
Merged

wb-tables DataTable Utilities #2438

merged 11 commits into from
Nov 5, 2024

Conversation

bozzit
Copy link
Contributor

@bozzit bozzit commented Oct 24, 2024

Utility Plugin that adds data formatting options and Table Totals

as Per Documentation: https://bozzit.github.io/GCWeb/m%C3%A9li-m%C3%A9lo/2024-10-datatable-utilities/index-eng.html

@Garneauma
Copy link
Contributor

Pre-approved upon successful review and passing of the méli-mélo checklist.

To do (for WET-BOEW team):

  • Create meta.md file
  • Create permanent URL

Copy link
Contributor

@Garneauma Garneauma left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution @bozzit ! Below are a few comments and suggestions:

  • Please remove all debug messages (or comment out).
  • Please rename index-eng.html to index-en.html and index-fra.html to index-fr.html to keep it consistent with the rest of the project.
  • Suggestion: create a separate documentation page for all the options instead of having them in the JS file. You could follow the table under "Configuration options" in the Fieldflow component. No need to have it translated.
  • Please add an implementation plan item: "Produce accessibility conformance report".
  • I don't see a working example for the following option: wb-col-cur-thousand.

@bozzit
Copy link
Contributor Author

bozzit commented Oct 31, 2024

for Debug messages you mean : debugMsg(... ? Those only get printed IF you pass data-wb-tables-utility="{"debug":true, to the component. I can amend the example to not use debug: true if you wish but I believe that having the ability to print debug messages on demand is a nice to have.

Let me know

@Garneauma
Copy link
Contributor

for Debug messages you mean : debugMsg(... ? Those only get printed IF you pass data-wb-tables-utility="{"debug":true, to the component. I can amend the example to not use debug: true if you wish but I believe that having the ability to print debug messages on demand is a nice to have.

Let me know

OK no problem about the debug messages. Sorry did not notice it was only with that option.

@bozzit
Copy link
Contributor Author

bozzit commented Nov 1, 2024

So once I have made the requested Changes, how do I go about submitting the changes? New Pull request? OR is there a way to update it?

@Garneauma
Copy link
Contributor

@bozzit You should make your changes locally. Then, you can amend your previous commit and force-push to your remote branch:

  • git add .
  • git commit --amend --no-edit
  • git push -f origin master

@Garneauma
Copy link
Contributor

@bozzit Could you please add a link from the documentation to the working example and vice-versa?

@bozzit
Copy link
Contributor Author

bozzit commented Nov 4, 2024

So there was a link already to the configuration from index, so I added a Back link at the top of the config pages.

Copy link
Contributor

@Garneauma Garneauma left a comment

Choose a reason for hiding this comment

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

@bozzit Thank you for creating the documentation page and for taking the time to translate it! I have highlighted some changes to keep it consistent with the rest of the GCWeb documentation and some small typos.

Note: not mandatory, but every instance of "Table" in French should be changed to "Tableau".

@bozzit
Copy link
Contributor Author

bozzit commented Nov 4, 2024

I think I got them all, but let me know if I missed or messed something up

Copy link
Contributor

@Garneauma Garneauma left a comment

Choose a reason for hiding this comment

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

@bozzit You missed a few. I have added some other ones.

@bozzit
Copy link
Contributor Author

bozzit commented Nov 4, 2024

Maybe email me the change files ? I’ll take a look at it in the morning.

@Garneauma
Copy link
Contributor

@bozzit The unresolved conversations are the only changes left to do. Please make sure to reveal hidden conversations by clicking the "Load more..." button.
Here's the list:

  • Add lang="en" on French documentation table.
  • Update paragraph above configuration table in both languages.
  • Remove useless <div class="mrgn-bttm-lg"> wrapping the first example in both languages.

I will then do a final review, but I think that's it!

Copy link
Contributor

@Garneauma Garneauma left a comment

Choose a reason for hiding this comment

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

Approved! Thank you for your contribution @bozzit.

Could you please update your PR comment (the one at the top of the page) to include the following (no need to be very detailed):

  • Clear and simple explanation of the new feature;
  • The impact on the sponsored department;
  • The impact on the public.

@bozzit
Copy link
Contributor Author

bozzit commented Nov 5, 2024

I thought I included this in the text at the top:

DataTable Utilities, includes Data Manipulation Classes for emails, Urls and Money. Also Includes Datatable Footer Totals.

Having this plugin integrated will make the transition to Canada.ca easier as these features are already in use on https://www.tpsgc-pwgsc.gc.ca, currently some of the pages that make use of this plugin and other pages that make use of Other JavaScripts are just pointed to from Canada.ca

The Public at Large, the plugin makes DataTables more user friendly with Clickable Links, and they are used to this functionality our PRE Canada.ca Pages.

@Garneauma
Copy link
Contributor

I thought I included this in the text at the top:

DataTable Utilities, includes Data Manipulation Classes for emails, Urls and Money. Also Includes Datatable Footer Totals.

Having this plugin integrated will make the transition to Canada.ca easier as these features are already in use on https://www.tpsgc-pwgsc.gc.ca, currently some of the pages that make use of this plugin and other pages that make use of Other JavaScripts are just pointed to from Canada.ca

The Public at Large, the plugin makes DataTables more user friendly with Clickable Links, and they are used to this functionality our PRE Canada.ca Pages.

Oh, that is my fault. I didn't realize the text in the documentation was meant for that. Sorry about that.

@Garneauma Garneauma merged commit 7215962 into wet-boew:master Nov 5, 2024
1 check passed
SebastianBurke added a commit to SebastianBurke/GCWeb that referenced this pull request Nov 27, 2024
Feedback area: adding a11y assessment performed by CRA (wet-boew#2433)

Patch - Méli-mélo: DataTable Utilities (wet-boew#2438)

* Initial Commit of 2024-10-datatable-utilities

* Adjustment to the jeckell html pages as the Javascript Didn't load

* Update to the meta.md

* Fix to toFrenchMoney Function failing under  'use strict' and Some fixes to the Sample HTML pages

* Fix for French Totals coming back as NaN

* Implemented wet-boew team Accesibility changes to sample pages + added css to replace <strong> on the Totals

* 1.1.0 - Wet-Boew Fixes + French Translation of Examples Page

* Fixed a Typo

* Added the URL Column to the Raw Data

* Updated Implementation Plan

* Fixed typo in implementation Plan

---------

Co-authored-by: Steve Bourgeois <[email protected]>

Patch - GC Home Page: Reduced Doormats and added all services button (wet-boew#2434)

* GC Home Page: Reduced Doormats and added all services button

* Home: updating All services button styling

---------

Co-authored-by: Marc-André Garneau <[email protected]>

Patch - Steps Méli-mélo: Update steps.css (wet-boew#2441)

* Update steps.css

Clear and simple explanation of the change/update:

Additional CSS and example added to support basic use (for numbers 2 to 10) of the start attribute for ordered (ol) lists as the CSS design uses a CSS count method to display the numbers and not the generated numbers from the html element.

Future design could include a javascript to allow any start number to exist.

The impact on the sponsored department (CRA) for that change/update:

No impact

The impact on the public for that change/update:

More flexibility of using the design in various situations.

* Update index.html

Added extended functionality example for use of start attribute for steps

* Update index-fr.html

Added extended functionality example using start attribute for steps

* Update steps-doc-en.html

Added start attribute code

* Update steps-doc-fr.html

Added code snippet for start attribute

* Update meta.md

Added updated feature to list

* Méli-mélo 2021-05-steps - Add working example for custom starting step

* Méli-mélo 2021-05-steps - Typo fix for custom starting steps

* Méli-mélo 2021-05-steps - Typo fix for custom starting steps

---------

Co-authored-by: Pierre Dubois <[email protected]>

Release v15.7.1 (wet-boew#2443)

Patch - Update _base.scss (wet-boew#2444)

* Update _base.scss

Clear and simple explanation of the change/update:

Updated input elements to be invisible simply by using opacity: 0 and z-index:2 in order for inputs to be selected by Dragon Naturally Speaking adaptive technology voice reader

Added fallback to show focus of checkboxes and radio buttons and selection of radio buttons in Windows high contrast mode.

The impact on the sponsored department (CRA) for that change/update:

Improved accessibility for users of the adaptive technology or visual mode

The impact on the public for that change/update:

No change for current use cases, but allows easier selection of radio button and checkboxes using Dragon Naturally Speaking, and provides proper focus in high contrast mode when using keyboard and selecting radio buttons

* removed test provisional class extension

* adding margin so input is nested inside design

* Adding documentation and fixing hidden checkbox hover

---------

Co-authored-by: Marc-André Garneau <[email protected]>

Requested changes complete
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.

3 participants