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

Convert the "Contact Leasing Agent" section to a "Management Company" section #389

Merged
merged 20 commits into from
Aug 12, 2021

Conversation

anders-schneider
Copy link

@anders-schneider anders-schneider commented Aug 11, 2021

Issue

Addresses #329

  • This change addresses the issue in full
  • This change addresses only certain aspects of the issue
  • This change is a dependency for another issue
  • This change has a dependency from another issue

Description

This change converts the "Contact Leasing Agent" section into a section with all the information we have about the "Management Company". (This follows #329 (comment) and the mocks; the "contact property to apply" will be a separate section.)

This change introduces two new pieces of information (when they are present): the management company name and the management website. These are displayed underneath the manager name, phone number, and email address (which were all there previously).

This change also removes the "high call volume" text since that's not in our mocks.

Screenshots:

  • When all the data (except email address is present):

image

  • When the property manager name is absent:

image

  • When the management company and website is absent (or if we were to flip showManagementCompany to false):

image

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Prototype/POC (not to merge)
  • This change is a refactor/address technical debt
  • This change requires a documentation update
  • This change requires a SQL Script

How Can This Be Tested/Reviewed?

I checked the following views for three different listings with different values for these new fields:

  • Desktop View
  • Mobile View

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have reviewed the changes in a desktop view
  • I have reviewed the changes in a mobile view
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have assigned reviewers
  • I have updated the changelog to include a description of my changes

@anders-schneider
Copy link
Author

Hey @willrlin I'd appreciate your feedback on whether this is the right way to keep ui-components as in-sync with Exygy as possible? There are some weirdnesses (e.g. t("leasingAgent.contact") set to "Management Company") but I think this change minimizes the deviation from Exygy

@anders-schneider
Copy link
Author

@tomgebauer Would you mind taking a look particularly at the issue where the website address overflows the box? I wonder if there are better ways to handle that (I was thinking of we could just put the text "website" or "management company website" and having that link to the actual website, but it also feels nice to show the actual url of what the person is about to click on.)

@tomgebauer
Copy link

@anders-schneider I think having the text "Website" and linking it to the URL is the simplest solution here. The heading of the module already tells the user that it's the management company, and if they really want to they can copy the URL using right-click. Is there a way top have it open in a new tab when it's clicked?

@anders-schneider
Copy link
Author

@tomgebauer

@anders-schneider I think having the text "Website" and linking it to the URL is the simplest solution here. The heading of the module already tells the user that it's the management company, and if they really want to they can copy the URL using right-click. Is there a way top have it open in a new tab when it's clicked?

Sounds good! I've updated the screenshots above - let me know if those look like what you had in mind. And made the change to make it so that clicking "website" will open a new tab.

Copy link

@willrlin willrlin left a comment

Choose a reason for hiding this comment

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

Please remember to open a PR to contribute the ui-components portion upstream.

@willrlin
Copy link

Since that string is supplied by leasing agents, could you also open a task to validate it on input and maybe also validate it here?

@anders-schneider
Copy link
Author

Since that string is supplied by leasing agents, could you also open a task to validate it on input and maybe also validate it here?

Done! #400

@anders-schneider anders-schneider merged commit 5f83660 into main Aug 12, 2021
@anders-schneider anders-schneider deleted the aeschneider-contact-leasing-agent-section branch August 12, 2021 19:50
anders-schneider added a commit that referenced this pull request Aug 16, 2021
… section (#389)

* Add management company and website to UI

* Change to "website" text and globe icon

* Revert changes to locales/xx.json (except "website")

* Formatting fixes

* Override leasingAgent.contact

* Switch to overriding dueToHighCallVolume

* Guard showing management company by a prop

* Add a test

* Fix code style issues with Prettier

* Make management company website link open a new tab

* Pass in management company details in the prop

* Make managementCompany optional, fix a warning

* Fix code style issues with Prettier

* Clarify test

* Rationalize checking for website

* Only prepend http if it's not already present

Co-authored-by: Lint Action <[email protected]>
seanmalbert pushed a commit that referenced this pull request Jun 23, 2022
… section (#389)

* Add management company and website to UI

* Change to "website" text and globe icon

* Revert changes to locales/xx.json (except "website")

* Formatting fixes

* Override leasingAgent.contact

* Switch to overriding dueToHighCallVolume

* Guard showing management company by a prop

* Add a test

* Fix code style issues with Prettier

* Make management company website link open a new tab

* Pass in management company details in the prop

* Make managementCompany optional, fix a warning

* Fix code style issues with Prettier

* Clarify test

* Rationalize checking for website

* Only prepend http if it's not already present

Co-authored-by: Lint Action <[email protected]>
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.

Improvements to the "Contact Leasing Agent" section of the individual-listing view
4 participants