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

Problem: customizing tabular view is hard #595

Closed
wants to merge 3 commits into from

Conversation

gotcha
Copy link
Member

@gotcha gotcha commented Apr 8, 2021

Solution: use views for table cells such that they can be overridden

! This is a PR on 1.4.x branch, iow Plone 5.1 branch. If merged, I will forward port it to master (Barceloneta LTS).

Solution: use views for table cells such that they can be overridden
@mister-roboto
Copy link

Thibaut Born your emails are not known to GitHub and thus it is impossible to know if you have signed the Plone Contributor Agreement, which is required to merge this pull request.

Learn about the Plone Contributor Agreement: http://docs.plone.org/develop/coredev/docs/contributors_agreement_explained.html How to add more emails to your GitHub account: https://help.github.com/articles/adding-an-email-address-to-your-github-account/

@mister-roboto
Copy link

@gotcha thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

Solution: fix pinned versions, do not force installing ride and reload
for robotframework
Solution: add it
@mister-roboto
Copy link

Thibaut Born your emails are not known to GitHub and thus it is impossible to know if you have signed the Plone Contributor Agreement, which is required to merge this pull request.

Learn about the Plone Contributor Agreement: http://docs.plone.org/develop/coredev/docs/contributors_agreement_explained.html How to add more emails to your GitHub account: https://help.github.com/articles/adding-an-email-address-to-your-github-account/

@ThibautBorn ThibautBorn reopened this Apr 12, 2021
@mister-roboto
Copy link

Thibaut Born your emails are not known to GitHub and thus it is impossible to know if you have signed the Plone Contributor Agreement, which is required to merge this pull request.

Learn about the Plone Contributor Agreement: http://docs.plone.org/develop/coredev/docs/contributors_agreement_explained.html How to add more emails to your GitHub account: https://help.github.com/articles/adding-an-email-address-to-your-github-account/

@mister-roboto
Copy link

@gotcha thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

@ThibautBorn ThibautBorn reopened this Apr 12, 2021
@mister-roboto
Copy link

Thibaut Born your emails are not known to GitHub and thus it is impossible to know if you have signed the Plone Contributor Agreement, which is required to merge this pull request.

Learn about the Plone Contributor Agreement: http://docs.plone.org/develop/coredev/docs/contributors_agreement_explained.html How to add more emails to your GitHub account: https://help.github.com/articles/adding-an-email-address-to-your-github-account/

@mister-roboto
Copy link

@gotcha thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

@ThibautBorn ThibautBorn reopened this Apr 13, 2021
@mister-roboto
Copy link

Thibaut Born your emails are not known to GitHub and thus it is impossible to know if you have signed the Plone Contributor Agreement, which is required to merge this pull request.

Learn about the Plone Contributor Agreement: http://docs.plone.org/develop/coredev/docs/contributors_agreement_explained.html How to add more emails to your GitHub account: https://help.github.com/articles/adding-an-email-address-to-your-github-account/

@mister-roboto
Copy link

@gotcha thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

@ThibautBorn ThibautBorn reopened this Apr 14, 2021
@mister-roboto
Copy link

Thibaut Born your emails are not known to GitHub and thus it is impossible to know if you have signed the Plone Contributor Agreement, which is required to merge this pull request.

Learn about the Plone Contributor Agreement: http://docs.plone.org/develop/coredev/docs/contributors_agreement_explained.html How to add more emails to your GitHub account: https://help.github.com/articles/adding-an-email-address-to-your-github-account/

@mister-roboto
Copy link

@gotcha thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

@@ -76,6 +76,7 @@ def album_folders(self):
"""
return self._album_results['folders']

@property
Copy link
Member

Choose a reason for hiding this comment

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

This breaks for anyone who currently calls view.tabular_fields() in custom code.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right !

Copy link
Member

@mauritsvanrees mauritsvanrees left a comment

Choose a reason for hiding this comment

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

The idea seems good. I wonder if this impacts performance negatively. But if this is in a batch of 20 items per page, this should be no problem.

But why for Plone 5.1? This Plone version is not supported anymore and will not get a release. I am okay with occasionally doing a bugfix release of an individual package, but not with a big change like this.

I would hesitate to put this in Plone 5.2 as well (branch 2.2.x).

So I would say: make a PR for master instead.

@ale-rt
Copy link
Member

ale-rt commented Apr 15, 2021

Just a note: after the @@title view was added to Plone I saw cases where traversal like here/title was returning the @@title view instead of the title attribute.
I never inspected if that was project specific and I just replaced that with python: here.title.
So I would suggest to not call the views Title or Creator but maybe add a reasonable suffix or prefix, e.g. tabular_view.Title or whatever you see fit.
That would be also easier to grep.

@gotcha
Copy link
Member Author

gotcha commented Apr 15, 2021

@mauritsvanrees Thanks for clarifications on where to make the PR.

@ale-rt I was hesitant on the view names, thanks for pointing me to potential issues.

@ale-rt
Copy link
Member

ale-rt commented Apr 15, 2021

Thanks to you for your brilliant work through the years!

@gotcha
Copy link
Member Author

gotcha commented Jun 22, 2021

I have ported the PR to master/ Plone 6. See #606

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.

5 participants