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

Jump to bottom

Added website and contact_role fields to businesses #397

Merged
merged 18 commits into from May 2, 2022
Merged

Added website and contact_role fields to businesses #397

merged 18 commits into from May 2, 2022

Conversation

scottharvey
Copy link
Contributor

@scottharvey scottharvey commented May 1, 2022 •
edited by joemasilotti

Pull request checklist

  • My code contains tests covering the code I modified
  • I linted and tested the project with bin/check
  • I added significant changes and product updates to the changelog

@scottharvey
Copy link
Contributor Author

Screen Shot 2022-05-01 at 09 28 08

Screen Shot 2022-05-01 at 09 29 22

Here is my first pass at implementing this feature, here are some things I think we should add:

  1. Client side URL validation in admin
  2. Make it so the business website opens in a new tab
  3. Maybe add a label to the show business page so it's clear what the contact role information is
  4. System tests

@scottharvey scottharvey mentioned this pull request May 1, 2022
Closed
@joemasilotti
Copy link
Owner

Looking good! Do you have the appetite to add those 4 items? If so, go for it. If not, let me know and we can hand it off.

joemasilotti
joemasilotti reviewed May 1, 2022
View reviewed changes
Gemfile Outdated Show resolved Hide resolved
app/models/business.rb Outdated Show resolved Hide resolved
db/migrate/20220501021745_add_website_url_and_contact_role_to_businesses.rb Outdated Show resolved Hide resolved
app/views/businesses/show.html.erb Outdated Show resolved Hide resolved
@joemasilotti
Copy link
Owner

Looking good! I have a few tweaks I want to make to the design that I'm hoping to get to soon.

I'm thinking of something like this.

image

@scottharvey
Copy link
Contributor Author

I'm thinking of something like this.

That looks nicer 😄

Let me know if you want me to do anything else for this PR 😉

@scottharvey scottharvey changed the title Added website_url and contact_role fields to businesses Added website and contact_role fields to businesses May 2, 2022
@joemasilotti joemasilotti merged commit 6a51850 into joemasilotti:main May 2, 2022
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joemasilotti joemasilotti joemasilotti left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@scottharvey @joemasilotti