collective / Products.PloneKeywordManager

Change, merge and delete keywords (AKA tags or subjects) in Plone.
https://pypi.org/project/Products.PloneKeywordManager/
4 stars 8 forks source link

UI Refresh and Tackle the slowness #48

Closed flipmcf closed 2 years ago

flipmcf commented 2 years ago

see https://community.plone.org/t/plonekeywordmanager-maintenance/14477/5

issue #17 issue #45

jensens commented 2 years ago

I need some days to get this done, but I am going to put your branch on the test server of a client project with many keywords that needs much management and then I'll come back with a review. But I am already sure it will be awesome!

flipmcf commented 2 years ago

Well, I tagged Issue #49 on also. We need that for our install.

I feel like I'm slowly scope creeping this PR. sorry.

petschki commented 2 years ago

Hey great to see some love on KeywordManager! Need this here too.

I'd stretch the scope of this PR even a bit more and implement Github Actions like for example here https://github.com/collective/collective.taxonomy/blob/master/.github/workflows/main.yml

flipmcf commented 2 years ago

Fine. "While we're at it".

I'd like to create a feature that highlights the white space in keywords. Currently, it's impossible to tell that "foo" and " foo" and "foo " are different keywords when shown on the page (and which one is the correct one to merge to) I'll create a new enhancement ticket to talk about it.

51

flipmcf commented 2 years ago

@petschki

Hey great to see some love on KeywordManager! Need this here too.

I'd stretch the scope of this PR even a bit more and implement Github Actions like for example here https://github.com/collective/collective.taxonomy/blob/master/.github/workflows/main.yml

is this #37 ?

flipmcf commented 2 years ago

And I hope I can mission creep even more:

52

petschki commented 2 years ago

@petschki

Hey great to see some love on KeywordManager! Need this here too. I'd stretch the scope of this PR even a bit more and implement Github Actions like for example here https://github.com/collective/collective.taxonomy/blob/master/.github/workflows/main.yml

is this #37 ?

yes it is

flipmcf commented 2 years ago
  • I think your PR targets Plone 6. Would it be possible to add some classes, so label positioning in Plone 5.2 works? I think its not much more.

I'm targeting my 5.2 install actually. I'm not going for 6. (should I? I don't want to.)

Adding classes to labels is not a big deal, but I do want to be on the same page strategically if plone 6 is a concern.

jensens commented 2 years ago

@agitator told me it should be possible to have code targeting both, so he may explain? poke

flipmcf commented 2 years ago

In review, this PR will address:

17 - the main reason for the PR - performance and UX with unusually large numbers of keywords

45 - the empty ( '' ) keyword is not displayed.

49 - make keyword manager work for subsites: 'localhost:8080/Plone/english'

37 - update CI (todo)

51 - Make keyword whitespace visible

52 - redirect url after form post goes back to original query and page

53 - portal status message fixed

62 - update SearchableText index when editing indexes

And also the untracked enhancement that adds the keyword cardinality next to the keyword

In the effort to draw a line in the sand and end the scope creep, I will not address (but I'm noting)

let's leave some work for 3.0.5 and get a release out.

jensens commented 2 years ago

@flipmcf Please mark as ready for review and press merge! Or is something missing? I do not think so.

jensens commented 2 years ago

@flipmcf as you fiddle with Github actions: The buttons on top of README.rst need some love too.

flipmcf commented 2 years ago

Please mark as ready for review and press merge! Or is something missing? I do not think so.

I'd like to get the workflow and README.rst buttons working as the previous travis CI did.

We're running tests, but coverage and black are not running yet.

Also, this is the first time I'm diving into readme badges - I'm going to have to learn a little today about that.

flipmcf commented 2 years ago

Coveralls updated and working with google CI. (wipes forehead)

https://coveralls.io/github/collective/Products.PloneKeywordManager?branch=flipmcf/issue-17

jensens commented 2 years ago

Thanks for the awesome work @flipmcf !