Skip to content

Conversation

@aleksfl
Copy link
Contributor

@aleksfl aleksfl commented Dec 1, 2025

Scope and purpose

Fixes #1650 .

Contributor Checklist

Every pull request should have this checklist filled out, no matter how small it is.
More information about contributing to Argus can be found in the
Development docs.

  • Added a changelog fragment for towncrier
  • Added/amended tests for new/changed code
  • Linted/formatted the code with ruff and djLint, easiest by using pre-commit
  • The first line of the commit message continues the sentence "If applied, this commit will ...", starts with a capital letter, does not end with punctuation and is 50 characters or less long. See our how-to
  • If this results in changes in the UI: Added screenshots of the before and after

@aleksfl aleksfl linked an issue Dec 1, 2025 that may be closed by this pull request
@github-actions
Copy link

github-actions bot commented Dec 1, 2025

Test results

    4 files    524 suites   19m 38s ⏱️
  623 tests   622 ✅ 1 💤 0 ❌
2 492 runs  2 488 ✅ 4 💤 0 ❌

Results for commit c0610b8.

♻️ This comment has been updated with latest results.

@codecov
Copy link

codecov bot commented Dec 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.37%. Comparing base (f8c6430) to head (c0610b8).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1652      +/-   ##
==========================================
- Coverage   79.44%   79.37%   -0.07%     
==========================================
  Files         129      129              
  Lines        6017     6032      +15     
==========================================
+ Hits         4780     4788       +8     
- Misses       1237     1244       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@@ -1,4 +1,5 @@
<ul class="z-20 menu menu-md dropdown-content bg-base-200 text-base-content rounded-box z-[1] mt-3 w-52 p-2 shadow max-h-svh">
<li>Argus version: {{ version }}</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the version is too visible if put at the top, and seeing as it is a "tertiary" action, it is probably best put at the bottom.

Here is a suggested menu item (with a divider on top):

<li>
  <div class="divider divider-secondary pl-2 pr-4 my-0 pointer-events-none"></div>
  <button class="text-xs" onclick="navigator.clipboard.writeText('{{ version }}')">
      <div class="space-y-2">
          <span>Argus version:</span>
          <span class="truncate">{{ version }}</span>
      </div>
      <i class="fa-solid fa-copy"></i>
  </button>
</li>

Which looks like this:
Image

Copy link
Contributor Author

@aleksfl aleksfl Dec 2, 2025

Choose a reason for hiding this comment

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

bilde

Looks alright with the release version, and with the dev version as well:
bilde

Although the copy button seems to be pushed away when I have local changes:
bilde

Copy link
Contributor

Choose a reason for hiding this comment

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

If we ever have a version number like that in prod, that's the bug, not how it is rendered :)

@aleksfl aleksfl marked this pull request as ready for review December 2, 2025 09:16
Copy link
Contributor

@hmpf hmpf left a comment

Choose a reason for hiding this comment

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

One liner fixes...

@github-project-automation github-project-automation bot moved this from 📋 Backlog to ♻ Changes requested in Argus development, public Dec 2, 2025
@hmpf
Copy link
Contributor

hmpf commented Dec 2, 2025

What will it look like when not logged in?

@aleksfl
Copy link
Contributor Author

aleksfl commented Dec 2, 2025

What will it look like when not logged in?

At the moment it is not present at all when not logged in due to the user menu requiring an authenticated user.
Might be an idea to include the version in the login page, but that might also be unnecessary.

@hmpf
Copy link
Contributor

hmpf commented Dec 2, 2025

When not logged in (check with user.is_authenticated in the template), put it to the left of the login button in the login template.

If anyone complains we can move it later (but what we need is an actual about-page.. out of scope.)

@johannaengland
Copy link
Contributor

Yes, I agree that it should also be visible somewhere when not logged in - in the case that we ever break the login mechanism

Copy link
Contributor

@johannaengland johannaengland left a comment

Choose a reason for hiding this comment

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

Only nitpicks here about the code, looks-wise I'm happy :)

@aleksfl
Copy link
Contributor Author

aleksfl commented Dec 2, 2025

When not logged in (check with user.is_authenticated in the template), put it to the left of the login button in the login template.

If anyone complains we can move it later (but what we need is an actual about-page.. out of scope.)

Not quite what you suggested but would something like this work?
bilde

@johannaengland
Copy link
Contributor

Not quite what you suggested but would something like this work? bilde

I think that looks good!

@hmpf hmpf self-requested a review December 2, 2025 10:54
@aleksfl
Copy link
Contributor Author

aleksfl commented Dec 2, 2025

Redid the login page version a bit, making it clearly separate from the login buttons. Also made it react to being hovered over so it is clearly clickable:
bilde

@aleksfl aleksfl requested a review from Simrayz December 2, 2025 11:19
Copy link
Contributor

@Simrayz Simrayz left a comment

Choose a reason for hiding this comment

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

Starting to look good, just some small suggestions now :)

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 2, 2025

@johannaengland johannaengland moved this from ♻ Changes requested to 👍 Reviewer approved in Argus development, public Dec 2, 2025
@hmpf hmpf added the rc-next Strong candidate for backend's next release label Dec 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rc-next Strong candidate for backend's next release

Projects

Status: 👍 Reviewer approved

Development

Successfully merging this pull request may close these issues.

Add version number of argus to argus frontend somewhere

5 participants