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

Add Timezone to Cert #1292

Merged
merged 17 commits into from
Sep 16, 2024
Merged

Add Timezone to Cert #1292

merged 17 commits into from
Sep 16, 2024

Conversation

Vzor-
Copy link
Contributor

@Vzor- Vzor- commented Sep 9, 2024

Closes #1289

WIP

Copy support soon to come in a separate PR.
Old:
Screenshot 2024-09-08 at 11 37 34 PM

New:
Screenshot 2024-09-08 at 11 34 38 PM
Screenshot 2024-09-08 at 11 34 47 PM

@Vzor-
Copy link
Contributor Author

Vzor- commented Sep 9, 2024

It is not at all obvious that the date entry is intractable, but I could not find a clean way to make it apparent. Due to how tables are implemented, we can't even do the hand-cursor to show it is interactable without a bunch of hit-detection code.

@tresf
Copy link
Contributor

tresf commented Sep 9, 2024

When switching from EST to UTC, the selected row loses focus. This should be fixed.

Also, the selection affects rows not clicked, which makes sense, but is out of context.

@tresf
Copy link
Contributor

tresf commented Sep 10, 2024

When switching from EST to UTC, the selected row loses focus. This should be fixed.

Also, the selection affects rows not clicked, which makes sense, but is out of context.

Both of these are addressed in the latest code changes, thanks. After a code review, I'm concerned about the use of description in the Certificate.java class. I think the enum is an improvement over hard-coded string values, but I think these descriptions are better suited for a UI-related class.

@@ -126,7 +170,22 @@ public Component getTableCellRendererComponent(JTable table, Object value, boole

// First Column
if (value instanceof CertificateField) {
label = stylizeLabel(STATUS_NORMAL, label, isSelected);
Copy link
Contributor

Choose a reason for hiding this comment

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

@Vzor- do you mind explaining these changes? At a glance I would not expect the first column logic to change as a result of this PR. Perhaps it was due to some unreverted changes, but I would expect this logic to be largely untouched.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With the timezone added, the warning tag was getting cut off. I decided to move it to the left for clarity, and share the formatting color of the value, with the key.

@tresf
Copy link
Contributor

tresf commented Sep 13, 2024

@Vzor- thanks for the simplification. I've made some minor refactorings to simplify the timezone flag by stashing it in the enum field. This will have the side-effect of being remembered between reloading of certs into the table.

@tresf tresf merged commit de82740 into qzind:master Sep 16, 2024
6 checks passed
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.

Site Manager: Show timezone for cert expiration
2 participants