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

Show dates on hover in JWT decoder #847

Closed
wants to merge 1 commit into from

Conversation

micahmo
Copy link

@micahmo micahmo commented Jun 8, 2023

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • New feature or enhancement
  • UI change (please include screenshot!)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Internationalization and localization
  • Other (please describe):

What is the current behavior?

Issue Number: #670

What is the new behavior?

This PR adds the ability to show understandable dates in a tooltip when hovering over a timestamp in a decoded JWT.

As mentioned in the bug, my JavaScript and understanding of the architecture is pretty weak, so feel free to reject this if it doesn't fit the current model.

Other information

DevToys-JWT-Demo.mp4

Quality check

Before creating this PR:

  • Did you follow the code style guideline as described in CONTRIBUTING.md
  • Did you build the app and test your changes?
  • Did you check for accessibility? On Windows, you can use Accessibility Insights for this.
  • Did you verify that the change work in Release build configuration
  • Did you verify that all unit tests pass
  • If necessary and if possible, did you verify your changes on:
    • Windows
    • macOS (DevToys 2.0)
    • Linux (DevToys 2.0)

@veler veler linked an issue Jun 8, 2023 that may be closed by this pull request
@btiteux
Copy link
Collaborator

btiteux commented Jun 11, 2023

Hello, Thank you for working on this request.

I'm not really fan of the show on hover. I didn't add the human date format before, was looking for a way to display it without a hover maybe a small modal with a question mark to trigger it?

@veler
Copy link
Collaborator

veler commented Jun 11, 2023

Hello,

Thanks @micahmo for this PR. I will join @btiteux on this. While I congratulate you for challenging the Monaco editor, I have 2 concerns about this tooltip approach:

  1. There's no indication that passing the mice on the date will display any extra information. If I'm a new user and I'm not aware of it, I will likely never find it. In other words, it isn't very discoverable.
  2. I'm also concerned by the accessibility aspect. This information is likely hard to get through a tooltip for someone who don't use a mice, or someone with vision impairment.

I would suggest to display this information in a permanent manner, either:

  • By adding a comment in the JSON next to the timestamp (but I'm not 100% sure whether it can cause any issue. I admit I never used a JWT in production, @btiteux (or you?) may know).
  • Add some text fields in the UI to display this info.

The comment in the JSON might be enough, and might be the best approach from a layout aspect (this tool already has a quite complex UI), but again, I'm not sure whether this could cause any frustration or not. I'm opened to opinions. :)

@micahmo
Copy link
Author

micahmo commented Jun 12, 2023

Hi @btiteux and @veler, thank you for the feedback and good conversation!

I will briefly push back on the notion that new users will not discover the tooltip. Since this is the design used by (what I believe is) the most popular JWT decoding site, jwt.io, I think people might actually expect this behavior in DevToys. 😉

jwt.io.mp4

Regardless, I totally understanding needing a different mechanism for accessibility and discoverability.

I believe I can condense both of your suggestions as follows:

  • A modal dialog
    • Personally, I am not a fan of dialogs to the extent they can be avoided.
  • Inline JSON comments
    • I did implement something like that here. If we went with this approach, I would suggest a button to toggle the comments on/off so that the "clean" payload can be copied if needed. That said, this approach might not have the most longevity if we want to include more detailed descriptions of other claims as suggested in #607.
  • Additional UI fields
    • I like this idea the best, but I don't want the UI to become more cluttered. In 607, someone mentioned jwt.ms, which is new to me. It has a tabbed interface where you can toggle between the raw decoded token and the claims fields explained. Although this site is brand new to me, I already like its approach the best. Instead of tabs, we could have an action button next to Copy that toggles the "explanation mode".

image

So my questions are:

  • Are you completely against the tooltip altogether, or could we keep it as a convenient, no-click solution for people who are expecting that behavior (in addition to another mechanism)?
  • For the additional mechanism, do you have a preference based on the ideas presented above?

Thanks again!

@veler
Copy link
Collaborator

veler commented Jun 14, 2023

A modal dialog
Personally, I am not a fan of dialogs to the extent they can be avoided.
Inline JSON comments
I did implement something like that #670 (comment). If we went with this approach, I would suggest a button to toggle the comments on/off so that the "clean" payload can be copied if needed. That said, this approach might not have the most longevity if we want to include more detailed descriptions of other claims as suggested in #607.
Additional UI fields
I like this idea the best, but I don't want the UI to become more cluttered. In 607, someone mentioned jwt.ms, which is new to me. It has a tabbed interface where you can toggle between the raw decoded token and the claims fields explained. Although this site is brand new to me, I already like its approach the best. Instead of tabs, we could have an action button next to Copy that toggles the "explanation mode".

+1 to all of this! I completely agree with you.

Are you completely against the tooltip altogether, or could we keep it as a convenient, no-click solution for people who are expecting that behavior (in addition to another mechanism)?

I think I will stay against for now. Even though I understand a good part of the population may know about it on jwt.io, I'm not fan of it. In addition, we are bleeding some extra options / info the CodeEditor control which will likely be only used by the JWT tool and therefore feel like CodeEditor will have an option specifically for 1 little scenario, which sounds a tiny unfair.

I'm willing to re-evaluate this opinion in the future, perhaps for DevToys 2.0.

For the additional mechanism, do you have a preference based on the ideas presented above?

Yes! For DevToys 1.0 (and 2.0), I like the idea of an extra "Explanation" button. Although I'm not super enthusiastic about the name, but like the overall idea and can't think of anything else at the moment (maybe it will come to me later).

Regarding adding this "Explanation" button, how about the following mecanism:
To keep CodeEditor generic, we could add a ControlControl in the bar where the button appears. This ContentControl, tied to a new DependencyProperty, could display any kind of content (so not only button).

It could be use like this:

<CodeEditor>
    <CodeEditor.OptionBarAdditionalContent>
       <Button Content="Explanation" Click="OnClick"/>
    </CodeEditor.OptionBarAdditionalContent>
</CodeEditor>

This way, we could extend any other code editor in the future with extra options when it makes sense to do so.

What do you think?

@micahmo
Copy link
Author

micahmo commented Jun 19, 2023

Hey again, apologies for the delay on this! I tried a few different approaches and layouts, and I think this is the one I'd like to initially propose. Focusing just on the UI/UX, let me know what you think. If it's on the right path, I can clean things up and open a new PR. Thanks! I'm also happy to expoud on any of the design decisions (some of which were driven by technical limitations I hit).

DevToys-JwtInfo-Demo.mp4

@btiteux
Copy link
Collaborator

btiteux commented Jun 21, 2023

Hello, @micahmo I like the new Ui with the button to toggle more information (like the jwt.ms). And the more information when we click on the row is really nice.

What do you think @veler ?

@veler
Copy link
Collaborator

veler commented Jun 23, 2023

I love it! :D

@micahmo
Copy link
Author

micahmo commented Jun 23, 2023

Thanks for the feedback! I will close this PR in favor of the rework in #864. I'll leave my branch for posterity in case it helps with tweaking Monaco in the future.

@micahmo micahmo closed this Jun 23, 2023
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.

JWT Decoder show timestamp as a DateTime tooltip
3 participants