Add Keywordmanager utility, services & controlpanel #1
Conversation
davisagli
left a comment
davisagli
left a comment
There was a problem hiding this comment.
I reviewed the REST services, but not the keywordmanager tool or the frontend yet.
@jnptk Is the keywordmanager tool mostly copied from Products.PloneKeywordManager, or are there specific things you changed that you'd like me to check?
davisagli
left a comment
davisagli
left a comment
There was a problem hiding this comment.
It would be good to have functional tests for the API endpoints.
|
@davisagli how can I get the KeywordIndex a keyword is from to the |
|
@jnptk I would change the route for KeywordView to |
| <p className="description"> | ||
| <FormattedMessage | ||
| id="keyword-manager-description" | ||
| defaultMessage="The Keyword Manager allows you to maintain the keywords used in your intranet. Start by selecting the keyword field you want to manage. You can then sort, filter, rename, merge, or delete individual keywords." |
There was a problem hiding this comment.
This is an add-on that might also be used in sites which are not an intranet.
| defaultMessage="The Keyword Manager allows you to maintain the keywords used in your intranet. Start by selecting the keyword field you want to manage. You can then sort, filter, rename, merge, or delete individual keywords." | |
| defaultMessage="The Keyword Manager allows you to maintain the keywords used in your website. Start by selecting the keyword field you want to manage. You can then sort, filter, rename, merge, or delete individual keywords." |
| selectedKeys !== 'all' && selectedKeys?.size === 0 | ||
| } | ||
| > | ||
| <Icon name={replaceSVG} size="20px" /> |
There was a problem hiding this comment.
We need a better icon here. I thought this one would reload the listing.
| selectedKeys === 'all' | ||
| ? keywords.items?.map((kw) => kw.name) ?? [] | ||
| : [...(selectedKeys as Set<string>)], | ||
| ); |
There was a problem hiding this comment.
This didn't work when I tested today. The whole page reloaded. You may need to add type="button" or add event.preventDefault() (to avoid accidentally triggering form submission) or add event.stopPropagation() to prevent the event from bubbling up to container elements.
|
@jnptk Let me know if you'd like me to take a look at the failing tests. |
| """ | ||
| query = {indexName: keywords} | ||
| if context: | ||
| query["depth"] = 0 |
There was a problem hiding this comment.
@davisagli I added this so when we have a context we only delete on the context itself and not on children within that context with the same keyword. But we loose exactly that functionality (to be able to do bulk changes within a context) what do you think?
TODO: make this behavior consistent throughout the tools functions
|
@davisagli what do you think about my loading indicator solution (see 18641e1) any improvement suggestions or maybe even a whole different idea on how to handle this? |
2 participants
No description provided.