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

Feature/export pdf #119

Merged
merged 8 commits into from
Feb 23, 2022
Merged

Feature/export pdf #119

merged 8 commits into from
Feb 23, 2022

Conversation

annashamray
Copy link
Contributor

@annashamray annashamray commented Feb 21, 2022

Note: weasyprint can't process grid tags (open issue - Kozea/WeasyPrint#543), and no design for pdf documents was provided, so I made it very simple and table based

@annashamray annashamray added the wip Work in progress label Feb 21, 2022
@annashamray annashamray removed the wip Work in progress label Feb 21, 2022
@@ -68,7 +68,8 @@

&__item {
padding: var(--spacing-medium);
border-bottom: 1px solid var(--color-gray-light);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I separated it into two properties for weasyprint to be able to process it

@annashamray annashamray requested review from JostCrow and alextreme and removed request for JostCrow February 21, 2022 12:49
@alextreme
Copy link
Member

Reviewed this functionally:

  • The logo in the PDF seems squished (
    2022-02-23_12-05
    )
  • I noticed that when exporting the profile you see Birthday as untranslated. This is okay for now (I think I had a separate task for Vasilieos for this)
  • I noticed that you can export each action individually. However I expected to have the option to export all tasks in one go like when you're exporting your profile. Happy to have that as a separate task to allow this to be merged in.
  • I noticed that when exporting the actions the descriptionfield didn't take newlines into account when exporting the PDF, it was all on one line. Might be good to doublecheck this in other places too.
  • I wasn't able to create a samenwerkingsplan locally so had to add one via the admin, seems to be something with selecting the contacts (but could be my environment as it is working properly on the testenvironment:
    2022-02-23_12-07

All in all good to go but double-check the logo, you might need to give it a width and height explicitly (I've seen this problem before with embedded images).

Copy link
Member

@alextreme alextreme left a comment

Choose a reason for hiding this comment

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

Double-check a couple of aspects but nearly there!

@annashamray
Copy link
Contributor Author

@alextreme

I noticed that you can export each action individually. However I expected to have the option to export all tasks in one go like when you're exporting your profile. Happy to have that as a separate task to allow this to be merged in.

In the design there is an export button for each action (https://projects.invisionapp.com/share/Y211V6JRHPC4#/screens/461963822) so I thought that you should be able to export each action individually

Should I remove these buttons and replace them with one button to export the list of action?

@alextreme
Copy link
Member

@alextreme

I noticed that you can export each action individually. However I expected to have the option to export all tasks in one go like when you're exporting your profile. Happy to have that as a separate task to allow this to be merged in.

In the design there is an export button for each action (https://projects.invisionapp.com/share/Y211V6JRHPC4#/screens/461963822) so I thought that you should be able to export each action individually

Should I remove these buttons and replace them with one button to export the list of action?

No you can leave the individual buttons in, this was indeed as designed. So simply adding an option to export all actions in one PDF is sufficient (and as mentioned this can also be split off into a separate task, up to you what you prefer)

@alextreme
Copy link
Member

Happy with the state as-is, feel free to merge and to create separate tasks for the other remarks where necessary

@annashamray
Copy link
Contributor Author

  • logo is fixed
  • birth date localization works for me (see the screenshot)
    image
  • I'll create a separate PR for mass action export
  • line breaks are added for description field
  • I didn't manage to reproduce the error with creating plans locally, perhaps it is indeed connected to your empty contact list, but it doesn't related to the pdf export so I'll leave it for the future PR

Thank you for the careful review and testing!

@annashamray annashamray merged commit 5d01c71 into develop Feb 23, 2022
@annashamray annashamray mentioned this pull request Feb 23, 2022
JostCrow pushed a commit that referenced this pull request May 4, 2022
JostCrow pushed a commit that referenced this pull request May 4, 2022
@annashamray annashamray deleted the feature/export-pdf branch May 9, 2022 10:06
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.

2 participants