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

Refactor story page to summary page #5732

Merged
merged 11 commits into from
Sep 12, 2024

Conversation

manuelmeister
Copy link
Member

This PR introduces the ability to filter ContentNodes and display them in an summary.

Solves #5067

@manuelmeister manuelmeister added the deploy! Creates a feature branch deployment for this PR label Aug 17, 2024
Copy link

github-actions bot commented Aug 17, 2024

Feature branch deployment currently inactive.

If the PR is still open, you can add the deploy! label to this PR to trigger a feature branch deployment.

@manuelmeister manuelmeister marked this pull request as ready for review August 18, 2024 06:54
@manuelmeister manuelmeister requested a review from a team August 18, 2024 06:54
frontend/src/components/print/config/SummaryConfig.vue Outdated Show resolved Hide resolved
frontend/src/locales/de.json Outdated Show resolved Hide resolved
frontend/src/components/print/config/SummaryConfig.vue Outdated Show resolved Hide resolved
pdf/src/campPrint/summary/SummaryDay.vue Outdated Show resolved Hide resolved
Copy link
Contributor

@BacLuc BacLuc left a comment

Choose a reason for hiding this comment

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

please fix the lint warnings in the new code, then i will approve.

print/components/config/Toc.vue Show resolved Hide resolved
@BacLuc
Copy link
Contributor

BacLuc commented Aug 25, 2024

You still have lint warnings in new code, e.g. here:
image
please fix these

@manuelmeister
Copy link
Member Author

@BacLuc strange, it somehow didn't complain when I linted locally in the container.

@pmattmann
Copy link
Member

Even if I understand the idea and can see the technical 'beauty' in it, I still wonder whether all our users understand the function.
Wouldn't it be easier to create separate function blocks for the individual summaries?

A few detailed questions:

  • Should users be able to print the notes (without context) individually?
    Or does that rather tempt to use them in a strange way?
  • Should it be possible to print block destinations and block contents individually?
    Wouldn't a combination be practical here?

@pmattmann pmattmann added the Meeting Discuss Am nächsten Core-Meeting besprechen label Aug 26, 2024
@manuelmeister
Copy link
Member Author

Wouldn't it be easier to create separate function blocks for the individual summaries?

We could still do this afterwards, if we see the need for it.

  • Should users be able to print the notes (without context) individually?

What context du you mean?

Or does that rather tempt to use them in a strange way?

What bad could it do?

  • Should it be possible to print block destinations and block contents individually?

    Wouldn't a combination be practical here?

I don't understand what you mean. Can you give an example?

@pmattmann
Copy link
Member

Wouldn't it be easier to create separate function blocks for the individual summaries?

We could still do this afterwards, if we see the need for it.

Note sure. Migration from generic to concrete is not necessarily trivial.

  • Should users be able to print the notes (without context) individually?

What context du you mean?

In what case are 'all' notes meaningful without the other block information?
I guess that the notes are named specifically so that only some of them can be printed.
This is so specific that we can hardly take it into account when migrating from generic to concrete concept.
And it will probably be difficult to remove it completely.

Or does that rather tempt to use them in a strange way?

What bad could it do?

If we can't get rid of this print-block, we will have to be maintain forever.

  • Should it be possible to print block destinations and block contents individually?
    Wouldn't a combination be practical here?

I don't understand what you mean. Can you give an example?

Sorry, typo.
Should be LearningObjectives and LearningTopics.
Wouldn't be more practical to have printed both of them?

@manuelmeister
Copy link
Member Author

Core Meeting Decision

  • Only do this for 'Storycontext' & 'SafetyConsiderations'
  • Create two separate configs for the GUI
  • Remove instanceNameFilter. This way it will certainly be abused. We would rather implement a more sophisticated way of defining "categories"/"tag"

@manuelmeister manuelmeister removed the Meeting Discuss Am nächsten Core-Meeting besprechen label Aug 27, 2024
pdf/src/CampPrint.vue Outdated Show resolved Hide resolved
print/components/config/Story.vue Outdated Show resolved Hide resolved
@pmattmann
Copy link
Member

pmattmann commented Sep 5, 2024

There is one more problem - but it already existed before this PR.

If you have a content with the period selector in the print configurator, the periods are printed in the order in which the periods were clicked.

image

@manuelmeister manuelmeister added this pull request to the merge queue Sep 12, 2024
Merged via the queue into ecamp:devel with commit b1312a8 Sep 12, 2024
30 checks passed
@manuelmeister manuelmeister deleted the feature/print-overviews branch September 12, 2024 18:38
@BacLuc BacLuc mentioned this pull request Sep 12, 2024
@manuelmeister manuelmeister mentioned this pull request Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy! Creates a feature branch deployment for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants