-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Rename 'Work package settings' page as 'General' #16550
Merged
cbliard
merged 10 commits into
dev
from
impl/57528-dedicated-admin-section-for-progress-tracking
Sep 13, 2024
Merged
Rename 'Work package settings' page as 'General' #16550
cbliard
merged 10 commits into
dev
from
impl/57528-dedicated-admin-section-for-progress-tracking
Sep 13, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cbliard
force-pushed
the
impl/57528-dedicated-admin-section-for-progress-tracking
branch
from
September 4, 2024 12:21
8f9f0f4
to
1affbd5
Compare
I did not understand what `require_admin_and_render_template` was supposed to do. I thought it was creating some initial user and rendering the template as a before action, but actually it only adds some examples. Using RSpec shared examples makes intent clearer.
The warning message is not yet implemented.
There is also a new helper to display HTML content in forms.
cbliard
force-pushed
the
impl/57528-dedicated-admin-section-for-progress-tracking
branch
from
September 11, 2024 16:42
2073a23
to
61d2daa
Compare
cbliard
force-pushed
the
impl/57528-dedicated-admin-section-for-progress-tracking
branch
from
September 12, 2024 09:36
8ef787b
to
a88260c
Compare
This is to display the warning when the user changes the calculation mode and then reloads the page, or when the user changes the calculation mode, navigates away and then goes back to the settings page.
3 tasks
spec/controllers/admin/settings/aggregation_settings_controller_spec.rb
Outdated
Show resolved
Hide resolved
aaron-contreras
suggested changes
Sep 12, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job! Some updates to the lookbook docs. Everything else looks and feels solid! 🌴
Co-authored-by: Aaron Contreras <[email protected]>
@aaron-contreras I addressed all your points. Thanks for the thorough review. |
cbliard
deleted the
impl/57528-dedicated-admin-section-for-progress-tracking
branch
September 13, 2024 07:17
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ticket
implementation ticket https://community.openproject.org/wp/57528
What are you trying to accomplish?
In Administration -> Work packages:
A new lookbook pattern page describes how to use forms: http://localhost:3000/lookbook/pages/patterns/forms
Screenshots
Before, progress calculation mode was inside the "Work package settings" administration page:
After, the page has been renamed "General" and the progress tracking settings have been put apart and primerized:
What approach did you choose and why?
I didn't just renamed the page title, I also renamed the menu name, controller name, and route name for consistency.
For primerizing the progress tracking form, some new helpers have been developed to make it easier to define forms when dealing with settings and to also allow mixing them with normal html tags or components. This is all documented in the form patterns page in the lookbook: http://localhost:3000/lookbook/pages/patterns/forms
Merge checklist