-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Use component for global and project definition administration #18522
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
Use component for global and project definition administration #18522
Conversation
37d6ae4
to
a1cb5b9
Compare
This comment was marked as outdated.
This comment was marked as outdated.
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.
Looks and works nicely, thank you Jens.👍
Just left two comments, regarding an outdated comment and one question.
Feel free to merge.
# TODO: This component is currently not in use! It should be a shared component | ||
# between the Projects::Settings::LifeCycleSteps::StepComponent and the | ||
# Settings::ProjectLifeCycleStepDefinitions::RowComponent. | ||
# It should hold the icon, definition name and gate text information. |
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.
I think this comment is not relevant anymore.
# TODO: This component is currently not in use! It should be a shared component | |
# between the Projects::Settings::LifeCycleSteps::StepComponent and the | |
# Settings::ProjectLifeCycleStepDefinitions::RowComponent. | |
# It should hold the icon, definition name and gate text information. |
Projects::PhaseDefinitionComponent.new( | ||
definition, | ||
edit_link: User.current.admin?, | ||
phase_text_options: { classes: "filter-target-visible-text" } |
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.
I like how the "filter-target-visible-text"
is logically separated to be context aware, but I'm unsure if it is required, because we seem to use in all the instances. Do you think we will use this component without the "filter-target-visible-text"
in the future?
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.
@dombesz I don't know. I just felt that it is not the responsibility of that component to be ready to be filterable with that controller.
Ticket
https://community.openproject.org/wp/62709
What are you trying to accomplish?
Use a common component on both the global as well as the project specific administration to render the information about the project phase definition:
There shouldn't be visible changes.