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

RSDEV-224 Cosmetic tweaks to audit page #85

Merged

Conversation

rlamacraft
Copy link
Contributor

This change includes:

  • Removing spaces between words and commas
  • Consistently applying link to name of filter, and not the conjunctive words
  • Formats time according ISO 8601

Copy link
Contributor

@mKowalski256 mKowalski256 left a comment

Choose a reason for hiding this comment

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

Looks good, but I've noticed one incorrect id element after refactoring - makes 'users' filter not appearing for the PI.

Also, as the content of 'Time' column is now shorter, I believe the width of that column could be reduced from 25% to 20% here.

by <a href="#" id="oids" class="addRow"><spring:message code="audit.identifiers.label"/></a>,
by <a href="#" id="dates" class="addRow"><spring:message code="audit.date.label"/></a>
and
by <a href="#" id="dates" class="addRow"><spring:message code="audit.users.label"/></a>
Copy link
Contributor

Choose a reason for hiding this comment

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

should be id="users" in this line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, yep, fixed

@rlamacraft
Copy link
Contributor Author

Also, as the content of 'Time' column is now shorter, I believe the width of that column could be reduced from 25% to 20% here.

Only down to 22% as otherwise it wraps where there's a long timezone offset

@mKowalski256 mKowalski256 self-requested a review August 14, 2024 15:02
Copy link
Contributor

@mKowalski256 mKowalski256 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@rlamacraft rlamacraft merged commit 8061f33 into rspace-os:main Aug 15, 2024
2 checks passed
@rlamacraft rlamacraft deleted the rsdev-224-auditing-tweaks branch August 15, 2024 10:06
@github-actions github-actions bot locked and limited conversation to collaborators Aug 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants