update styles & modify html.erb to function with the new styles #17

Merged
merged 3 commits into from Jul 13, 2018

Conversation

Projects
None yet
2 participants
@chrisbohn8
Collaborator

chrisbohn8 commented Jul 8, 2018

The updated styles should be ready to merge into the master branch. Feel free to test on the styles-testing branch before you pull into the master. Let me know if I need to make any other changes.

@stephenyeargin

Most of my notes are about the test suite, which can be run via bundle exec rake test.

We can also remove bulma from the Gemfile since it's no longer in use (with bundle install run afterwards to drop it from Gemfile.lock).

- <span class="has-text-grey-light">/</span>
- $<%= guest_beer.price %>
+
+ <!-- Wine & Cider List -->

This comment has been minimized.

@stephenyeargin

stephenyeargin Jul 9, 2018

Member

This version of the application does have a data model for these items, though they are not exposed in ActiveAdmin yet. Not really sure if we should opt to include them in the view and ditch the "Guest Beers" object (already a misnomer).

@stephenyeargin

stephenyeargin Jul 9, 2018

Member

This version of the application does have a data model for these items, though they are not exposed in ActiveAdmin yet. Not really sure if we should opt to include them in the view and ditch the "Guest Beers" object (already a misnomer).

- $<%= guest_beer.price %>
+
+ <!-- Wine & Cider List -->
+ <div id="wine-cider" class="beer">

This comment has been minimized.

@stephenyeargin

stephenyeargin Jul 9, 2018

Member

Tests will fail here as well, because it will see 12 .beer classes (11 from the fixtures, 1 here) instead of the expected 11.

assert_select '.beer', 11

@stephenyeargin

stephenyeargin Jul 9, 2018

Member

Tests will fail here as well, because it will see 12 .beer classes (11 from the fixtures, 1 here) instead of the expected 11.

assert_select '.beer', 11

@@ -1,7 +1,7 @@
<!DOCTYPE html>
<html>
<head>
- <title>Beer List :: East Nashville Beer Works</title>
+ <title>ENBW Beer List</title>

This comment has been minimized.

@stephenyeargin

stephenyeargin Jul 9, 2018

Member

Previous review omitted this comment:

This change will break the test suite, as seen here. We would likely want to update it accordingly.

assert_select 'title', 'Beer List :: East Nashville Beer Works'

@stephenyeargin

stephenyeargin Jul 9, 2018

Member

Previous review omitted this comment:

This change will break the test suite, as seen here. We would likely want to update it accordingly.

assert_select 'title', 'Beer List :: East Nashville Beer Works'

@stephenyeargin

This comment has been minimized.

Show comment
Hide comment
@stephenyeargin

stephenyeargin Jul 13, 2018

Member

We discussed this offline last night, and this good to merge for now. The outlined tasks can be addressed in future PRs.

Member

stephenyeargin commented Jul 13, 2018

We discussed this offline last night, and this good to merge for now. The outlined tasks can be addressed in future PRs.

stephenyeargin added some commits Jul 13, 2018

@stephenyeargin stephenyeargin merged commit 8d78ad4 into master Jul 13, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@stephenyeargin stephenyeargin deleted the styles-testing branch Jul 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment