Skip to content

Conversation

@rrrooommmaaa
Copy link
Contributor

@rrrooommmaaa rrrooommmaaa commented Dec 17, 2021

This PR displays signature and encryption badges as per the task

close #4201


Tests (delete all except exactly one):

  • Tests added or updated

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

@rrrooommmaaa rrrooommmaaa force-pushed the issue-4201-signature-encryption-badges branch from a7020c0 to 0a38b68 Compare December 19, 2021 13:08
@rrrooommmaaa
Copy link
Contributor Author

I have implemented and tested these "encryption" 3 cases when we actually have message text:

  • encrypted (green, if decrypted succesfully)
  • decrypt error: security hazard (red) - for bad_mdc and no_mdc
  • not encrypted (red)

In other cases we can leave the error message as it was implemented before: as we don't have message text, we don't need badges, do we?

All signature badges are implemented given the decryption succeeds. if decryption returns "security hazard", then "not signed" badge is shown.

@rrrooommmaaa rrrooommmaaa changed the title replaced signature info with badge replaced signature info with badge, also added encryption badge Dec 19, 2021
@rrrooommmaaa
Copy link
Contributor Author

@limonte Can you please review my CSS, as this "click to retry" message isn't multi-lined too nicely
image

@rrrooommmaaa rrrooommmaaa force-pushed the issue-4201-signature-encryption-badges branch from 0a38b68 to fd7a281 Compare December 19, 2021 13:32
@rrrooommmaaa rrrooommmaaa requested a review from limonte December 19, 2021 13:48
@rrrooommmaaa rrrooommmaaa marked this pull request as ready for review December 19, 2021 13:48
@tomholub
Copy link
Collaborator

I have implemented and tested these "encryption" 3 cases when we actually have message text:

* encrypted (green, if decrypted succesfully)

* decrypt error: security hazard (red) - for bad_mdc and no_mdc

* not encrypted (red)

In other cases we can leave the error message as it was implemented before: as we don't have message text, we don't need badges, do we?

All signature badges are implemented given the decryption succeeds. if decryption returns "security hazard", then "not signed" badge is shown.

I guess so.

@tomholub
Copy link
Collaborator

tomholub commented Dec 20, 2021

@rrrooommmaaa limonte will be moving on to a role elsewhere starting next year, not sure if he still has time left this year.

Please try to remove all of the css that make it float on the right. Instead, there should be position:relative and the badges should be on the left size, as if forming the first line of the text. Then the width of the container they are in should be full line (standard div with no css). That should fix your issue too.

@rrrooommmaaa
Copy link
Contributor Author

Please try to remove all of the css that make it float on the right. Instead, there should be position:relative and the badges should be on the left size, as if forming the first line of the text. Then the width of the container they are in should be full line (standard div with no css). That should fix your issue too.

Is it looking better now?
image

@tomholub
Copy link
Collaborator

Somewhat, not really. I'll take a look

@tomholub
Copy link
Collaborator

tomholub commented Dec 24, 2021

It's the max-width: 300px; causing it.

An easy way to debug that is a right click on the badge and "inspect element". It will show which css rules apply to that element.

image

There you can uncheck or change individual items until you are happy with them.

I'll do some fiddling with the css and submit a commit.

@tomholub
Copy link
Collaborator

Tried the functionality - looks good. After testing it, here below, and for all decrypt errors, I would still add a "decrypt error" red badge.

image

I would like user to get trained to always expect some sort of a badge there and look for it (in case they care).

I'll file another issue for that.

@tomholub tomholub enabled auto-merge (squash) December 24, 2021 12:29
Copy link
Collaborator

@tomholub tomholub left a comment

Choose a reason for hiding this comment

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

excellent

@tomholub tomholub merged commit a808b6b into master Dec 24, 2021
@tomholub tomholub deleted the issue-4201-signature-encryption-badges branch December 24, 2021 12:34
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.

update signature+encryption rendering to use badges

3 participants