OSS Diff #219
OSS Diff #219
Conversation
…one, but made progress with it.
Issue #4 changed
…ng my code to dev_devayani instead of a pull request.
Made changes in my branch "dev_luis" to satisfy #3
Dev joel 2
… which had the download and copy buttons. The download and copy buttons are added in "Get a New Refresh Token" and "Access Token".
Deleted the Tokens Section and Implemented the Copy function.
Merge updates from dev_devayani branch into main
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive redesign of the RERUM API authentication interface, transforming it from a single-page workflow to a more structured, user-friendly experience with improved organization and enhanced functionality.
- Restructured the UI layout with responsive columns and a collapsible "Checks" section for managing authentication tokens
- Enhanced user experience with better error handling, confirmation dialogs, and automatic token management
- Added new features including token download/copy functionality and improved status tracking
| .column { | ||
| float: left; | ||
| width: 33%; | ||
| } | ||
|
|
||
| .column2 { | ||
| float: right; | ||
| width: 9%; |
There was a problem hiding this comment.
Using float for layout is outdated and can cause layout issues. Consider using CSS flexbox or grid instead for more reliable and modern layout management.
| .column { | |
| float: left; | |
| width: 33%; | |
| } | |
| .column2 { | |
| float: right; | |
| width: 9%; | |
| .columns-container { | |
| display: flex; | |
| flex-direction: row; | |
| width: 100%; | |
| } | |
| .column { | |
| flex: 0 0 33%; | |
| /* width: 33%; */ | |
| } | |
| .column2 { | |
| flex: 0 0 9%; | |
| /* width: 9%; */ |
| If the token you have is not working, it may be because access tokens expire every 30 days. You can use | ||
| your refresh token to get a new access token. | ||
| </p> | ||
| <textarea class="form-control" id="a_t" placeholder="Your access token goes here."></textarea> |
There was a problem hiding this comment.
The same element ID 'a_t' is used in multiple places (lines 233 and 278). This creates duplicate IDs which violates HTML standards and can cause JavaScript functionality issues.
| <textarea class="form-control" id="a_t" placeholder="Your access token goes here."></textarea> | |
| <textarea class="form-control" id="a_t_test" placeholder="Your access token goes here."></textarea> |
| if (window.history.replaceState) { | ||
| const cleanUrl = window.location.protocol + "//" + window.location.host + window.location.pathname; | ||
| window.history.replaceState({}, document.title, cleanUrl); | ||
| } |
There was a problem hiding this comment.
[nitpick] The window.history.replaceState check is unnecessary in modern browsers. All browsers that support the History API also support replaceState. Consider removing the conditional check.
| if (window.history.replaceState) { | |
| const cleanUrl = window.location.protocol + "//" + window.location.host + window.location.pathname; | |
| window.history.replaceState({}, document.title, cleanUrl); | |
| } | |
| const cleanUrl = window.location.protocol + "//" + window.location.host + window.location.pathname; | |
| window.history.replaceState({}, document.title, cleanUrl); |
| const copyText = $("#a_t").val() | ||
| try { | ||
| await navigator.clipboard.writeText(copyText); | ||
| alert("Copied the text: " + copyText); | ||
| } catch (err) { | ||
| console.error('Failed to copy: ', err); | ||
| alert('Failed to copy text.'); | ||
| } |
There was a problem hiding this comment.
Displaying the full access token in the alert message (line 499) could expose sensitive credentials if the screen is being recorded or viewed by others. Consider showing a generic success message instead.
| const copyText = $("#new_refresh_token").val() | ||
| try { | ||
| await navigator.clipboard.writeText(copyText); | ||
| alert("Copied the text: " + copyText); | ||
| } catch (err) { | ||
| console.error('Failed to copy: ', err); | ||
| alert('Failed to copy text.'); | ||
| } |
There was a problem hiding this comment.
Displaying the full refresh token in the alert message (line 510) could expose sensitive credentials if the screen is being recorded or viewed by others. Consider showing a generic success message instead.
5 participants
For the review process. More to come.