-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add results page #6
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
Conversation
rise-erpelding
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionally, this works! I can navigate from the welcome panel to the results panel and back again. I left one question about whether we still want the welcome panel open or not (it seemed like maybe not from the validation instructions?).
Also going to note that if I test this out with a smaller viewport, ie my 13" laptop screen, while Firefox (pictured) gives me a scrollbar on the welcome panel so I can access the button, with Chrome, I don't have a scrollbar and can't access the button at all. I didn't notice this on main because the welcome panel seemed to be just large enough, but it seems to be a little bigger on this branch, I think with the spacing from welcome.css now being applied? This may be out of scope for this ticket and might be worthy of a follow-up bug ticket!
| analyzeBtn.textContent = 'Analyze this website'; | ||
| }, 2000); | ||
| analyzeBtn.addEventListener('click', async () => { | ||
| await extensionManager.openPanel('results'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noting that the button from the welcome panel (and all of the other welcome panel elements) are still in the DOM when I'm on the results panel because the welcome panel is still open, would we want to remove those? Maybe over here when we open the results panel, also close the panel? (I think that may visually animate the welcome panel to slide out then slide in the results panel though, is that undesirable?)
5182615 to
29b6495
Compare
|
📦 Extension packages built successfully! |
|
@rise-erpelding Thanks for taking a look! It seemed like the The one-panel-at-a-time problem seems to be solved, too. Another thing to note is that on <div class="cv-welcome__icon">
🌱
</div>.cv-panel--welcome__icon {
font-size: 3rem;
margin-bottom: 1rem;
}I did fix that here (and went a bit out of scope), which might explain the disrepancy. |
rise-erpelding
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! I'm able to use the buttons to navigate from the welcome panel to the results panel, and the elements on each are separate!
This work adds a results panel that will eventually display analysis results. The end user can navigate between the results panel and welcome panel.
Relevant links
To validate
feat--add-results-pagenpm iandnpm run build