bcgov / entity

ServiceBC Registry Team working on Legal Entities
Apache License 2.0
23 stars 57 forks source link

PPR PDF OUTPUT - UX Assurance Feedback Changes #7885

Closed PatrickAHeath closed 2 years ago

PatrickAHeath commented 2 years ago

There are a number of needed visual design updates to be made to the PPR outputs to align them with the final Visual and UX Design. Please walk through on a call with Tracey before starting.

doug-lovett commented 2 years ago

Working from: REGULAR VERSION - Invision Design Comps: https://projects.invisionapp.com/share/R810MDBHVE3P#/screens?browse

Comments after completing UX assurance changes:

https://app.zenhub.com/files/157936592/6934c78f-31d4-4e68-9432-c190a3fa0c69/download

tlebedovich commented 2 years ago

@doug-lovett - There seems to be outstanding UX Assurance still to do based on the feedback PDF from the Design Ticket -- and the recent example outputs you provided are not using BC Sans font anymore (they were before, but now they seem to be using Verdana). Can you re-export these using BC Sans?

Let's have a call and review and decide what to do about the duplicate headers etc.

The original UX Assurance Feedback is contained in the PDF in this ticket: https://app.zenhub.com/workspaces/entity-5bf2f2164b5806bc2bf60531/issues/bcgov/entity/7344

doug-lovett commented 2 years ago

@tlebedovich I need to investigate why the custom font is not being used generating PDF's locally. A sample dev environment report is attached. https://app.zenhub.com/files/157936592/3035cde8-e722-4d8c-876e-2e41517cb261/download

tlebedovich commented 2 years ago

@doug-lovett - the font in your sample dev environment report is also not BC Sans?

Screen Shot 2021-07-28 at 9.17.54 AM.png

yuisotozaki commented 2 years ago

@doug-lovett Please find attached UX Assurance notes added to the dev sample PDF.

UX Assurance Notes July 28

Outstanding questions

doug-lovett commented 2 years ago

@yuisotozaki Attached is the example serial number search updated from your previous feedback. https://app.zenhub.com/files/157936592/ce8fd7b4-9304-4676-8380-39fafa9a01bc/download

I have also added some comments to your UX assurance notes. This example was generated locally and uses the BC Sans fonts. https://app.zenhub.com/files/157936592/fe2a3206-dc5c-4bac-9065-95b2779de743/download

To answer the first outstanding question, yes, we can have Page x of y in the footer.

yuisotozaki commented 2 years ago

@doug-lovett

Confirmed that the locally generated PDF has BC Sans ✅

Observation Notes for anyone else looking at this issue later.

A couple more tweaks. In the comp below, I'm referring to this visual design for white space reduction in the subsections. https://projects.invisionapp.com/d/main/#/console/21259632/448624145/inspect 7885 ux assurance 1

This is my fault for having the comment box so close to both in the last round. The request for "bold" was only for the first line and not both. 7885 ux assurance 2

doug-lovett commented 2 years ago

@yuisotozaki Hello again, the sample attached below has the changes from your latest review. This version will go to dev today if you have no further updates.

https://app.zenhub.com/files/157936592/dce481ac-f904-4e60-9c99-8e4c3f3efc80/download

yuisotozaki commented 2 years ago

@doug-lovett Here's my review result.

doug-lovett commented 2 years ago

@yuisotozaki Update attached again. Here are my comments.

https://app.zenhub.com/files/157936592/b720f382-1503-4fbf-895c-19919b000ed4/download

yuisotozaki commented 2 years ago

@doug-lovett It's getting there! I think you're doing all the things correctly in the source but the tool isn't producing the correct font sizes. Do you know if there's any kind of scaling effect being applied anywhere - I'm suspecting scaling because all the fonts are smaller by a similar ratio.

image

doug-lovett commented 2 years ago

@yuisotozaki Yes there is some scaling. I was following the invision css using px. This latest version switches to pt for all font sizes.

https://app.zenhub.com/files/157936592/44be1217-7adf-4f6e-9208-b3ed78ed3329/download

yuisotozaki commented 2 years ago

@doug-lovett We're so close!

doug-lovett commented 2 years ago

@yuisotozaki One more update attached, sorry about all these iterations.

https://app.zenhub.com/files/157936592/75f74034-352c-4d91-982f-81e6ef0cd51d/download

yuisotozaki commented 2 years ago

@doug-lovett I think we got it! 🎉 ✅ Thank you so much for all the redo's! Hope you have a great long weekend! 🏖️

doug-lovett commented 2 years ago

@yuisotozaki Thanks for your patience. Have a great long weekend too! Starting right now.

yuisotozaki commented 2 years ago

@doug-lovett The last bit for this PDF is the inclusion of Agreement Type and Act in the section headers of the registrations. Simply include these two fields with the other registration details in the grey area. For very long Agreement Type values, we want to wrap just in the right hand side.

EDIT: Updated Registration Type value to title cap.

image

tlebedovich commented 2 years ago

hi @yuisotozaki - Before Doug builds this;

Let's not make these all caps - it can get hard to read. And I think we should confirm the titles with @PatrickAHeath or @janisrogers. It maybe should be "Financing Agreement Type"? And I'm not sure what "Act" should be, but if it was a longer title it would look better :)

janisrogers commented 2 years ago

@tlebedovich @yuisotozaki In the current system the Agreement type is called the 'Base Reg. Type', So I think 'Registration Type' would be a good name. The current system doesn't include the act as far as I can see. Do you know if this is a requirement?

yuisotozaki commented 2 years ago

@janisrogers I'm looking at these screenshots and SA doesn't have the Act but Repairers Lien is both a type and an act? ref 8087: https://docs.google.com/spreadsheets/d/1C3KYYmxj-22759YVu4o4DXNTsNYNapW1vvFgYZkzwjY/edit#gid=0

janisrogers commented 2 years ago

@yuisotozaki, @PatrickAHeath is going to send out an email to check what we should be using. It seems very inconsistent in the old system, but they may want us to stick with that. Stay tuned.

yuisotozaki commented 2 years ago

Based on the spreadsheet @PatrickAHeath provided from the existing system, there appears to be an if-statement that decides whether to show REGISTRATION_DESC or REGISTRATION_ACT value.

Patrick's recommendation:

If the intent is to display in the registration output the registration description and the act that it relates to, I’d suggest cleaning up the REGISTRATION_TYPES table and using the values for the matching registration type code. This may impact the descriptions displaying on existing registrations but the old format of the outputs will not be generated after we go live.

EDIT: There are no impacts to current system if we update the table.

This means: a) The values are stored in all-caps so design can't impact that. We don't want to change any database values to title caps as that will impact current system output. b) Cleaning-up the REGISTRATION_DESC to include the act would also mean impacting existing system so that's a no go.

My recommendation is to rename the fields to reflect what they are (Registration Description and Registration Act) and show the values as-is. Clean up can be done after go-live.

Changing existing database values, especially ones that are stored in all caps, means we need to spend time investigating any ripples effects in case they're soft-enumerated in the code.

@tlebedovich @janisrogers What do you think?

janisrogers commented 2 years ago

The existing system (the mainframe system) uses a different database, so changes would not affect the existing system. I don't know if they would impact the API. It might be worth getting input from Doug.

yuisotozaki commented 2 years ago

@doug-lovett Are registration type/description and registration act field values enumerated and used in conditional logic? If not, we can update the values in the table to title cap.

doug-lovett commented 2 years ago

@doug-lovett Currently the API (and report) values for financing statement registration type and act come from the registration_types table as is. As Janis and Patrick have identified, the values in this table can be modified with no impact to the legacy system or to any other process that I am aware of.

yuisotozaki commented 2 years ago

@doug-lovett Let's go with just the Registration Description label and value. I can create a new ticket for updating the database content so that we can close off this PDF ticket.

image

yuisotozaki commented 2 years ago

8486 created to clean up the registration_desc table content and re-introduce the 'Registration Act' field to the PDF.

janisrogers commented 2 years ago

@yuisotozaki Is this ready for QA, or is there still outstanding UX Assurance?

yuisotozaki commented 2 years ago

@janisrogers We need one last output from @doug-lovett with the Registration Description field included in the section headers.

doug-lovett commented 2 years ago

@janisrogers @yuisotozaki The latest example is attached. https://app.zenhub.com/files/157936592/8498ff0f-7fce-4dee-b35c-6abbf9b23d36/download

yuisotozaki commented 2 years ago

@doug-lovett ✅ This checks out!

Note: The sample has "Registration Type" as field label while the latest design mock uses "Registration Description". This is fine because we are drawing the value from registration_type only for now.

When we have the database values updated so that the registration_type field values are cleaned up, we can update the field label to "Registration Description".

chdivyareddy commented 2 years ago

Created a new ticket for the issues found #8517.

chdivyareddy commented 2 years ago

Please do not close this ticket until I verify the dependent ticket:)

jinghualicgi commented 2 years ago

Per RC discussion, I will close the tickets which are in Done pipeline. image.png