Move username validation from report output to the input field#420
Open
kaungmyatshwe1397 wants to merge 2 commits intofossasia:mainfrom
Open
Move username validation from report output to the input field#420kaungmyatshwe1397 wants to merge 2 commits intofossasia:mainfrom
kaungmyatshwe1397 wants to merge 2 commits intofossasia:mainfrom
Conversation
Contributor
Reviewer's GuideMoves username validation from the Scrum Report output area into the username input field, adds inline/accessible error handling and styling (including dark mode support), wires validation into all relevant UI flows, and removes the old scrum report error injection logic. Sequence diagram for generating a report with username validationsequenceDiagram
actor User
participant Popup as PopupJS
participant UsernameInput as platformUsername_input
participant Storage as chrome_storage_local
participant ScrumHelper as scrumHelper_allIncluded
User->>UsernameInput: Type username
User->>Popup: Click generateReport button
Popup->>Popup: handleGenerateReport()
Popup->>Popup: validateUsername(showError_true)
alt Username invalid
Popup->>UsernameInput: showUsernameValidationError()
Popup-->>User: Report generation blocked
else Username valid
Popup->>Storage: get(platform)
Storage-->>Popup: platform value
Popup->>Storage: get(startingDate, endingDate, orgName, orgRepo, ignoreOrgRepos, platformUsername)
Storage-->>Popup: stored values
Popup->>ScrumHelper: allIncluded(popup)
ScrumHelper-->>Popup: generated report HTML
Popup-->>User: Render Scrum Report
end
User->>UsernameInput: Press Enter key
UsernameInput->>Popup: keydown(Enter)
Popup->>Popup: handleGenerateReport() (same flow as above)
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
platformUsernamefield hasaria-describedbyandaria-invalidset both in HTML and immediately again in JS; consider defining these attributes in only one place to avoid redundancy and reduce the chance of them getting out of sync. - Exposing
validatePlatformUsernameInputonwindowjust soupdatePlatformUI/setPlatformDropdowncan call it introduces a brittle global; consider refactoring so the validation function is imported/shared as a module helper instead of being attached towindow.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `platformUsername` field has `aria-describedby` and `aria-invalid` set both in HTML and immediately again in JS; consider defining these attributes in only one place to avoid redundancy and reduce the chance of them getting out of sync.
- Exposing `validatePlatformUsernameInput` on `window` just so `updatePlatformUI`/`setPlatformDropdown` can call it introduces a brittle global; consider refactoring so the validation function is imported/shared as a module helper instead of being attached to `window`.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 Fixes
Fixes # <417>
📝 Summary of Changes
📸 Screenshots / Demo (if UI-related)
✅ Checklist
👀 Reviewer Notes
Add any special notes for the reviewer here
Summary by Sourcery
Validate the Scrum Helper username directly on the input field instead of in the generated report output, with accessible inline error messaging and styling.
Bug Fixes:
Enhancements: