Conversation
| export class ItemProjectAdminElement extends APIConsumerElement { | ||
| static properties = { | ||
| ...APIConsumerElement.properties, | ||
| urlSettings: { type: String, attribute: "url-settings" }, | ||
| }; | ||
|
|
||
| handleClick(event) { | ||
| const isAdmin = this.data?.permissions?.admin; | ||
| if (this.disabled || isAdmin === false) { | ||
| event.preventDefault(); | ||
| event.stopPropagation(); | ||
| return; | ||
| } | ||
|
|
||
| this.queueEvent(event); | ||
| } | ||
|
|
||
| render() { | ||
| const isAdmin = this.data?.permissions?.admin; | ||
| const isDisabled = this.disabled || isAdmin === false; | ||
| const urlSettings = this.data?.urls?.settings || this.urlSettings || "#"; | ||
| const label = this.label || msg(`Configure project`); | ||
|
|
||
| return html` | ||
| <a | ||
| class="ui button ${classMap({ | ||
| disabled: isDisabled, | ||
| loading: !isDisabled && this.state === States.LOADING, | ||
| })}" | ||
| href=${urlSettings} | ||
| @click=${this.handleClick} | ||
| data-content="${label}" | ||
| aria-label="${label}" | ||
| aria-disabled="${isDisabled}" | ||
| tabindex=${isDisabled ? -1 : 0} | ||
| > | ||
| <i class="fa-duotone fa-wrench icon"></i> | ||
| </a> | ||
| `; | ||
| } | ||
| } | ||
| customElements.define( | ||
| "readthedocs-item-project-admin", | ||
| ItemProjectAdminElement, | ||
| ); | ||
|
|
There was a problem hiding this comment.
Is there a reason why you implemented this as a new class instead of reusing the existing class? Pretty sure this is the only view the menu item is used.
There was a problem hiding this comment.
I think the code doesn't matter here since Eric was trying to show the idea only. He wrote:
Would love review on the concept rather than the code itself, since I haven't dived in there.
|
For background here, this has a dropdown context menu because there have been other items in this list previously. The context menu is helpful for items that have bad icon matches and would only be confusing to users in icon form. In the context menu, the text is explicit. We don't use text buttons in the listing because it eats space, so context menu serves these two purposes. If there are no other items to go in the context menu, it can be squashed. If we want other actions in the context menu, we should add them and keep the menu. What does this look like when the user is not an admin? |

Summary:
Testing:
Issues:
Copilot generated