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

Remove token from html #8

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

joshbaskaran
Copy link
Contributor

@joshbaskaran joshbaskaran commented Oct 23, 2024

Now keeps the token in memory and copies it over directly to the users' clipboard.

Fixes #9

@joshbaskaran joshbaskaran requested a review from kjellp October 23, 2024 10:56
@kjellp
Copy link
Contributor

kjellp commented Oct 23, 2024

I guess I'lI need to pull this branch to the tryggve server to see it action first. And I also realized that it is a highly user-facing feature we may need to inform about when we introduce it in prod :)

@joshbaskaran
Copy link
Contributor Author

I guess I'lI need to pull this branch to the tryggve server to see it action first. And I also realized that it is a highly user-facing feature we may need to inform about when we introduce it in prod :)

I've already tested it by setting up a mock server and seeing how the website responds to the incoming token. All good here. But yes, users definitely will assume that website broke cause they no longer see the token!

Let me know if you'd like some UI updates as well :)
Currently it simply removes it so it could potentially look a little 'out of place'

@kjellp
Copy link
Contributor

kjellp commented Oct 25, 2024

It's now deployed on the https://tryggve.tsd.usit.uio.no server/site. I can login, a few gotchas:

The "user-id-string" is not displayed anymore, not sure we want to remove that completely.

The copy button does not work for me. I click it, but the content does not seem to reach my clip-board. When I paste in a terminal window, I get the previous content instead. Can you test on the tryggve site?

@kjellp
Copy link
Contributor

kjellp commented Oct 25, 2024

And we probably want some visual representation of the token on the page. It feels a bit truncated and the previous users will expect "something" to appear. Maybe and icon, with a text "Your token is ready"or similar ?

joshbaskaran and others added 2 commits November 6, 2024 10:16
Added explicit content-type declaration in the fetch header.

Co-authored-by: Yasin <[email protected]>
Specify headers in fetch call.
- Streamline token interface with horizontal layout
- Add clear error state when token fetch fails
- Update button styling for better visual hierarchy
- Improve copy-to-clipboard feedback
- Remove redundant UI elements in error state
@joshbaskaran joshbaskaran linked an issue Nov 19, 2024 that may be closed by this pull request
Copy link
Contributor

@kjellp kjellp left a comment

Choose a reason for hiding this comment

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

We discussed the layout / webpage in the dev meeting last Friday, and we like the improvements, and had few suggestions to make it clearer that

  • the username and token are 2 different strings in the web page
  • both can and should be possible to copy out by the user
  • and that it should be intuitive which is which, and hard to mix up (copying one and thinking it's the other one)

So this may involve an additional button (copy "LS login userid string") in addition to the "LS Login access token", placed in a 2 by 2 layout with the user string and a token representation, making it clear which button copies what element to the clip-board. Happy to explain more if needed :)

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 UI to flow better with the removed token Hide token in fega website for security reasons.
2 participants