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

issue 411, adding Research button to navbar with existing icon #417

Merged

Conversation

abudri
Copy link
Collaborator

@abudri abudri commented Feb 18, 2020

Resolves #411

Description

Not sure that this involves any new added tests needed, please let me know. Just adding Research to navbar right after Design, using the existing Packing .svg image as a placeholder for now, per discussion with Mae.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Just tested in browser

image

Copy link
Collaborator

@h-m-m h-m-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, @abudri ! Thanks for attending the hack night last week and for looking into this issue. I'm sorry I wasn't able to respond earlier when you asked for help.

I'm happy to help with feedback and suggestions — I've included one below. Maybe we can sync up some time next week!

@@ -14,9 +14,13 @@
<img src="https://voices-of-consent-static-images.s3.amazonaws.com/Design+Stage.svg" title="Design">
<span>Design</span>
</a>
<a class="nav-icon" href="http://localhost:3000/box_requests?utf8=%E2%9C%93&filter_by=designed">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hostname should not be hard coded because this link will break if we deploy it to remote heroku servers. Since this is an ERB file, you can use the Rails view helpers to generate links that will get dynamically evaluated. There are some examples here:

<td><%= link_to 'Show', box %></td>
<td><%= link_to 'Edit', edit_box_path(box) %></td>
<td><%= link_to 'Destroy', box, method: :delete, data: { confirm: 'Are you sure?' } %></td>

@abudri abudri force-pushed the issue_411_research_button_to_navbar branch from d9e6a7f to a828053 Compare February 18, 2020 17:33
used route helper for box_requests_path for now as a placeholder, instead of a hardcoded url
@abudri abudri force-pushed the issue_411_research_button_to_navbar branch from a828053 to 36d880d Compare February 18, 2020 17:40
@@ -14,9 +14,13 @@
<img src="https://voices-of-consent-static-images.s3.amazonaws.com/Design+Stage.svg" title="Design">
<span>Design</span>
</a>
<a class="nav-icon" href="<%= box_requests_path %>">
Copy link
Collaborator

@h-m-m h-m-m Feb 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting here that @abudri and I talked about this — it might be nice to change all the links here to use <%= link_to %> instead of this current pattern, and that's something that can wait for a future PR

@maebeale maebeale merged commit ab0ed9c into rubyforgood:develop Mar 10, 2020
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.

Add Research button to navbar (use placeholder image for now)
3 participants