cds-snc / design-gc-conception

MIT License
4 stars 0 forks source link

Publish: Date input component #1137

Closed adorayi closed 1 month ago

adorayi commented 1 month ago

📇 User story

Publish the pieces needed for Date input component

âś… Definition of Done / Outcomes

đź“ś Acceptance criteria

đź“ť More info

This ticket only tracks the complete of this specific template.

daine commented 1 month ago

Changed the estimate to a 2 because there's no ticket for the new pages on the website (this is currently the ticket to update the website).

daine commented 1 month ago

Hi - here is the PR for the website code changes. Would appreciate a final review from design and translation @RavinaSamaroo @MatildePerrusclet TIA! French URL - https://pr-420.d35vdwuoev573o.amplifyapp.com English URL - https://pr-420.djtlis5vpn8jd.amplifyapp.com

MatildePerrusclet commented 1 month ago

All good except for the FR anatomy text (and the alt text I presume). It is updated in the document https://docs.google.com/document/d/1d2vi_gx3QKO_wP3TjdvwP-KIN0paV04OqyY8e6r2l80/edit?disco=AAABVjJrYB4&usp_dm=true

ClementineHahn commented 1 month ago

@daine Update PR with latest content today + last round of review and looking to publish soon, by end of sprint for sure.

RavinaSamaroo commented 1 month ago

New anatomy text proposed by Matilde is more descriptive, but is misaligned from the anatomy text used in similar components, like the input field. I've asked @SmartMouthWords to review and provide a decision on if we move forard with the new text or keep the old text. https://www.figma.com/design/7HNtNYzWNmcxjYtG8UlnBz/Internal-assets?node-id=1925-9533&node-type=symbol&t=wVdSaIOWBCNSxwlU-0

MatildePerrusclet commented 1 month ago

@RavinaSamaroo the new text was in the works since last February with you and Amy as editors, when reviewing I've only reorganized content.

Amy and I met to review this and @SmartMouthWords decision is to the keep the newest version and make the adjustments in Figma. We will also make sure to align 'Input' and the 'Text area' documentation and have opened a ticket #1156

daine commented 1 month ago

I have updated the PR with the new text and images. Here is the PR with detailed description & links to preview the changes: https://github.com/cds-snc/gcds-docs/pull/420

A few little things I've found that I've made changes on:

  1. In french, under "Composants connexes", I've commonly seen the pattern <component name> : <description>. An example is the input/champ de saise component having this: "SĂ©lecteur de date : lorsque vous". For consistency, I applied the same pattern on the PR, even thought the google doc doesn't have the " : ".
  2. Under "Composants connexes", we mention "SĂ©lecteur de date", but I have compared the english version in both Date Input and the regular Input components and we have two names for this in english: "Date picker" (found in Date input) and "Date selectors" (found in Input). I've left this as-is and I've followed the content in the document instead. Just flagging this if this is an inconsistency or not.
  3. In both english and french, most of our pages have this text: 3.1. Design and accessibility for <component name> | Accessibilité et design des <component name> instead of "Accessibility and Design", so upon discussing with Matilde, we are keeping that format for consistency. 3.2. Coding and accessibility for <component name> | Codage et accessibilité des <component name> instead of "Accessibility and Code Guidance" -- same as above
  4. For the component preview, there is nothing indicated on the google docs, but the figma file shows a few examples. I've added the two examples on the code preview to try and follow the prototype, so please check if this is good. I left out the "required" attribute as I don't see that as a common pattern for our other form component previews. Edit: noticed on more thing upon PR feedback, should we also change the legend to be different for the two examples on the preview page? At the moment they both say "Legend". 4.1. For your action: Should we change it to say different legends for the two examples?

MatildePerrusclet commented 1 month ago

Hi @daine ,

Please find my comments below, I’ve also added them to the ticket. Please note that I’m not 100% set up and I’m reviewing content on my Macbook only which isn’t optimal. I have a meeting with Techops to set up monitors this week so reviews will be more efficient.

  1. Adding “:” works for the Composants connexes section
  2. Thanks for flagging, I’ll make sure to discuss this with Amy to see if we need to make a change on the other components.
  3. Yes, confirmed, let’s keep it as it is for all the other components.
  4. It’s find if both examples show “Legend”, it’s straightforward and clear.

Other comments

daine commented 1 month ago

Thanks for confirming Matilde! I've updated the PR with changes to add the following:

Other comments

In regards to the partials - I think it's best to have a separate ticket for it. It does appear on all our other components as well, so it's not just date input that's currently affected by it. Good catch! Update new ticket numbers are: