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

feat: feedback link #148

Merged
merged 4 commits into from
Dec 12, 2024
Merged

feat: feedback link #148

merged 4 commits into from
Dec 12, 2024

Conversation

crfmc
Copy link
Collaborator

@crfmc crfmc commented Nov 19, 2024

  • 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
@crfmc crfmc requested a review from sehilyi December 10, 2024 16:25
@crfmc crfmc marked this pull request as ready for review December 10, 2024 16:25
src/App.css Outdated
@@ -40,9 +40,319 @@ a:hover {
text-decoration: 3px underline #5bb6ea;
}

/* NavigationBar styles */
.navigation-container {
Copy link
Member

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
Comment on lines 763 to 776
<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}
/>
Copy link
Member

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!

Copy link
Member

@sehilyi sehilyi left a 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
@crfmc crfmc requested a review from sehilyi December 11, 2024 19:54
@crfmc crfmc merged commit bbea2e8 into main Dec 12, 2024
14 checks passed
@crfmc crfmc deleted the crfmc/feedback_button branch December 12, 2024 18:32
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.

2 participants