-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Relation Manager Customisation Improvements #4444
Conversation
@danharrin I noticed in the changes that you've cleared out a fair few blank lines between lines 886 and 1000 of |
Hi @bennothommo, seems that VS Code played up and went mental with the new lines - my bad for not checking. Fixed the newline issues brought up by Travis CI, however some other errors have popped up now that look seemingly unrelated to this PR, mind having a look through them? |
@danharrin The code quality checks basically are limited to checking complete files - we can't limit it to specifically the lines of code that you have modified, so it's possible that it will bring up issues in parts of the code that have been there for while. Given that it is only one minor issue - this empty line here needs to be removed - would you mind just quickly removing that for us and then we'll get cracking with the testing? |
All done @bennothommo 👍 |
Cheers @danharrin. Just note this won't be out until we start development on build 458, as 457 is in feature freeze now. |
No worries @bennothommo :) |
All whitespace has been removed @LukeTowers |
All done @LukeTowers. |
Co-Authored-By: Ben Thomson <ben@abweb.com.au>
Those changes should be complete @LukeTowers 👍 |
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.
LGTM! Waiting on @bennothommo to review again.
@danharrin is there a PR to the docs documenting the new functionality? |
Not yet, is that something that needs doing before this can be merged? We could just prepare something before it's released into a version. |
@danharrin yup, needs to be ready to go before it gets merged otherwise it'll never get done. Speaking from experience here 😂 |
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.
Sorry @danharrin, just a couple more :)
modules/backend/behaviors/relationcontroller/partials/_toolbar.htm
Outdated
Show resolved
Hide resolved
modules/backend/behaviors/relationcontroller/partials/_toolbar.htm
Outdated
Show resolved
Hide resolved
Co-Authored-By: Ben Thomson <ben@abweb.com.au>
….htm Co-Authored-By: Ben Thomson <ben@abweb.com.au>
….htm Co-Authored-By: Ben Thomson <ben@abweb.com.au>
Co-Authored-By: Ben Thomson <ben@abweb.com.au>
Co-Authored-By: Ben Thomson <ben@abweb.com.au>
Co-Authored-By: Ben Thomson <ben@abweb.com.au>
All done @bennothommo. @LukeTowers, I've done the docs change, see octobercms/docs#444. |
This change allows for easy customisation of the relation manager titles and toolbar buttons.
For buttons - define your custom strings directly in the standard
toolbarButtons
syntax......and the old syntax still works...
For titles - define different strings for each mode (form, list or pivot)...
This feature provides developer convenience, for example in this situation where I have a relation manager that is used to assign user permissions to a project:
Before
After
This PR contains no breaking changes.