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.

Sign up for GitHub

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

Jump to bottom

Hide statute amendments checkbox #3440

Merged
merged 4 commits into from
Mar 15, 2024
Merged

Hide statute amendments checkbox #3440

merged 4 commits into from
Mar 15, 2024

Conversation

Elblinator
Copy link
Member

first step of #3085

@Elblinator Elblinator added this to the 4.2 milestone Mar 14, 2024
@Elblinator Elblinator marked this pull request as ready for review March 14, 2024 12:02
emanuelschuetze
emanuelschuetze reviewed Mar 14, 2024
View reviewed changes
Copy link
Member

@emanuelschuetze emanuelschuetze left a comment

Choose a reason for hiding this comment

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

It it really the right way to hide a settings option? In earlier versions we used a single line "hide" attribute for settings. Maybe we can support this?

@Elblinator
Copy link
Member Author

If I understand the Code correctly this option cannot simply be called but needs new implementing.
Given that we do not plan to hide settings in general, but just for the moment, I would prefer this temporary workaround

But I could misunderstand the code. @bastianjoel what do you think about this?

@jsangmeister
Copy link
Contributor

Why not simply remove the setting from the definition list? Seems cleaner than implementing this workaround. @emanuelschuetze the hide attribute was probably in OS3 - in OS4, this does not exist (yet).

@Elblinator
Copy link
Member Author

Why not simply remove the setting from the definition list?

Removing it in the definition-list makes it completely unusable. Other files need the definition to determine whether or not statutes are allowed

@jsangmeister
Copy link
Contributor

I meant from the meeting-settings-definition.ts. That only defines what is displayed where in which group and not which settings are generally available.

@jsangmeister
Copy link
Contributor

The previous suggestion did not work since the MeetingSettingsDefinitionService parses the definition list to determine which fields to request, which I did not know. I implemented a more general solution instead which can hide arbitrary entries, like @emanuelschuetze suggested. @Elblinator please review.

@Elblinator
Copy link
Member Author

Looks very nice and clean :)

@Elblinator Elblinator removed their assignment Mar 15, 2024
@Elblinator Elblinator merged commit 4f588b7 into OpenSlides:main Mar 15, 2024
3 checks passed
@Elblinator Elblinator deleted the 3085-hids-statute-amendment-checkbox branch March 18, 2024 07:39
peb-adr added a commit to peb-adr/openslides-client that referenced this pull request Mar 20, 2024
@peb-adr
Merge staging/4.1.5 into stable/4.1.x. Update 20240320
62417a7 
* commit 'faf4189d28f0b7e6b528a545b5893866d9235acc': (52 commits)
  Update translations (OpenSlides#3451) (OpenSlides#3454)
  Prevent some linter warnings and make OS4 more accessiblle (OpenSlides#3103)
  Change display of identical motions (OpenSlides#3425)
  Hide statute amendments checkbox (OpenSlides#3440)
  Test MeetingSettingDirective (OpenSlides#3437)
  Test TreeSortService (OpenSlides#3443)
  Update mdc dialog (OpenSlides#3397)
  Update all translations (OpenSlides#3417)
  Prevent certain speech state changes (OpenSlides#3395)
  Change label color of focused form field in dark mode (OpenSlides#3399)
  Improve service worker update flow (OpenSlides#3368)
  Update meta (OpenSlides#3392)
  Change motion delete warnings (OpenSlides#3373)
  Fix delete speakers from view model list (OpenSlides#3375)
  Make own structure_level changeable (OpenSlides#3374)
  Fix moderation note permission updates (OpenSlides#3365)
  Add staging branch to CI
  Add arrow back to contributions list (OpenSlides#3364)
  add username sort option to account list (OpenSlides#3434)
  Activate re-add button only if active speaker is true (OpenSlides#3380)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bastianjoel bastianjoel bastianjoel approved these changes

@emanuelschuetze emanuelschuetze emanuelschuetze approved these changes

Assignees

@Elblinator Elblinator

Labels
clean up
Projects
None yet
Milestone
4.2
Development

Successfully merging this pull request may close these issues.
4 participants
@Elblinator @jsangmeister @emanuelschuetze @bastianjoel