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

Issue(KUI-1383): update heading visibility logic to match admin tool preview #391

Conversation

allazis
Copy link
Contributor

@allazis allazis commented Aug 20, 2024

This PR contains

  • Updates for heading visibility logics to match preview logics in the admin tool in PR for KUI-1343.
  • Minor updates of components

Headings are only shown if

  • the heading is mandatory (having type mandatory or mandatoryAndEditable)
  • it has content and it is mandatory for some (having type mandatoryForSome)
  • it has content and it is stored as visible in the database (having visibleInMemo set to true in DB)

If possible when reviewing, please run project locally and compare with ref or kth.se. Example memos to use for comparison:

  • DH1623/DH162320241-61063
  • MJ2462/MJ246220222-1
  • II1306/II130620222-2 (differs because Detailed plan and subsection under standard content in Examination were shown before, although they should not)

E.g. http://localhost:3000/kurs-pm/MJ2462/MJ246220222-1 with https://www.kth.se/kurs-pm/MJ2462/MJ246220222-1

@allazis allazis marked this pull request as ready for review August 21, 2024 05:52
@allazis allazis requested a review from belanglos as a code owner August 21, 2024 05:52
@allazis allazis requested a review from a user August 21, 2024 05:52
Copy link
Contributor

@amirhossein-haerian amirhossein-haerian left a comment

Choose a reason for hiding this comment

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

LGTM, Well done, I really liked that you made the code cleaner. I think it is more understandable when you read the code now ⭐️
I tested the code locally as well and it seems great.

Copy link
Contributor

@axelbjo axelbjo left a comment

Choose a reason for hiding this comment

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

Looks good to me! Great improvements regarding variable names and structure

@allazis allazis merged commit a3ab047 into master Aug 21, 2024
3 checks passed
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