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

Tools module functionality #2951

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Tools module functionality #2951

wants to merge 15 commits into from

Conversation

alex-odysseus
Copy link
Contributor

Addressing #2330

There was a new widget-like view designed in contrast to the usual tabular views. The toggle button component defines if a Tool should be displayed to a standard User. Administrators should see all Tools display and change the Tool visibility

@@ -0,0 +1,103 @@
<heading-title params="name: ko.i18n('common.tools'), icon: 'toolbox', theme: 'dark'"></heading-title>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We may need some formatting applied to a few rows

@anthonysena anthonysena self-assigned this Dec 17, 2024
@anthonysena
Copy link
Collaborator

Just some notes on the content of this pull request based on my testing.

  1. When adding a new tool, the button "Apply" should instead be "Add" or "Save":

image

  1. The layout of this page feels a little awkward but it is functional. Some suggestions:
  • Move the edit/delete icons to appear along side the link title (assuming these are only shown to admins).
  • The entire area of the "tool" is clickable to take you to the URL of the tool in a new tab. Perhaps add some hover-over to show when the user is in an active area of the screen to note that it is clickable? Right now there is no hover-over behavior as I would expect when clicking a link.
  • I'm unclear if the created by and date are always displayed or is this only for admins?

image

Copy link
Collaborator

@anthonysena anthonysena left a comment

Choose a reason for hiding this comment

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

Please see suggested changes.

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.

3 participants