-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat: feedback link #148
feat: feedback link #148
Conversation
crfmc
commented
Nov 19, 2024
•
edited
Loading
edited
- Add feedback button to allow users to email
- Style buttons on navigation bar/vis-overview-panel navigation bar + interactions
- Restructure CSS
- Improve tab navigation for accessibility
-style: reorganize buttons -feat: new mail icon -feat: set variable to hold email address
-style: update navigation bar -feat: add menu icon -chore: add types
src/App.css
Outdated
@@ -40,9 +40,319 @@ a:hover { | |||
text-decoration: 3px underline #5bb6ea; | |||
} | |||
|
|||
/* NavigationBar styles */ | |||
.navigation-container { |
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.
Since we created a separate file for the navigation bar (src/ui/NavigationBar.tsx
), should we make a separate CSS file as well (e.g., ui/NavigationBar.css
)?
src/App.tsx
Outdated
<NavigationBar | ||
demo={demo} | ||
setShowAbout={setShowAbout} | ||
showSmallMultiples={showSmallMultiples} | ||
setShowSamples={setShowSamples} | ||
gosRef={gosRef} | ||
currentSpec={currentSpec} | ||
getHtmlTemplate={getHtmlTemplate} | ||
demoIndex={demoIndex} | ||
externalDemoUrl={externalDemoUrl} | ||
externalUrl={externalUrl} | ||
isChrome={isChrome} | ||
FEEDBACK_EMAIL_ADDRESS={FEEDBACK_EMAIL_ADDRESS} | ||
/> |
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.
Thank you for making this change! A lot cleaner than the previous inline implementation!
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.
I left only a comment on creating a separate CSS file. Otherwise, this looks good to me!
-style: match navigation bar styles for vis-overview-panel -style: add hover state for overview panel links -chore: separate navigation bar styles -chore: create css folder