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: creating views for public and partner lists #1271

Merged
merged 7 commits into from
May 19, 2022

Conversation

YazeedLoonat
Copy link
Collaborator

  • Addresses #issue
  • This change is a dependency for another issue

This address part 1 of that issue

Description

This creates 2 new listing views
one is specific to the public listings list, the other is specific to the partner listing list

How Can This Be Tested/Reviewed?

Head to https://detroit-public-dev.netlify.app/
verify that the listing data that is displayed on the listing list is what is expected

Head to https://detroit-partners-dev.netlify.app/sign-in
and do the same

for devs
I think this is easiest to test if you aim your local at prod or dev and pull up current prod and dev as well so you can quickly compare the result sets and what the front end looks like. They should look exactly the same

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 run yarn generate:client and/or created a migration if I made backend changes that require them
  • I have exported any new pieces added to ui-components
  • My commit message(s) is/are polished, and any breaking changes are indicated in the message and are well-described
  • Commits made across packages purposefully have the same commit message/version change, else are separated into different commits

Reviewer Notes:

Steps to review a PR:

  • Read and understand the issue
  • Review the code itself from a style point of view
  • Pull the changes down locally and test that the acceptance criteria is met
  • Also review the acceptance criteria on the Netlify deploy preview (noting that these do not yet include any backend changes made in the PR)
  • Either explicitly ask a clarifying question, request changes, or approve the PR if there are small remaining changes but the PR is otherwise good to go

On Merge:

If you have one commit and message, squash. If you need each message to be applied, rebase and merge.

@netlify
Copy link

netlify bot commented May 11, 2022

Deploy Preview for detroit-public-dev ready!

Name Link
🔨 Latest commit fa806d5
🔍 Latest deploy log https://app.netlify.com/sites/detroit-public-dev/deploys/628438d236750a0008c03136
😎 Deploy Preview https://deploy-preview-1271--detroit-public-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented May 11, 2022

Deploy Preview for detroit-partners-dev ready!

Name Link
🔨 Latest commit fa806d5
🔍 Latest deploy log https://app.netlify.com/sites/detroit-partners-dev/deploys/628438d2a54a100009481720
😎 Deploy Preview https://deploy-preview-1271--detroit-partners-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@@ -63,13 +63,15 @@ views.partnerList = {
select: [
"listings.id",
"listings.name",
"listings.applicationDueDate",
"property.id",
...getBaseAddressSelect(["buildingAddress"]),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this address and the one on 73?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

on the partner site's listing grid we do display the address for the listing, so we do want to have that data

Copy link
Collaborator

@emilyjablonski emilyjablonski left a comment

Choose a reason for hiding this comment

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

LGTM after one question!

@YazeedLoonat
Copy link
Collaborator Author

readd listing features

Copy link
Collaborator

@seanmalbert seanmalbert left a comment

Choose a reason for hiding this comment

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

@YazeedLoonat ,
The publicListings view doesn't work for filtered or favorites since we want the listing to show as it does on the initial listing browse page, so I think we can get rid of that view and use base for those cases.

FYI, I fixed an unrelated issue I noticed with listing features not showing on the list pages.

@YazeedLoonat
Copy link
Collaborator Author

Hey @seanmalbert I think we can keep the new views, I made a few updates if you want to take a peep but now filtered and favorite pages should functionally be the same as the main listings page

lemme know what you think or if you still think we should switch to only updating the base view

Copy link
Collaborator

@seanmalbert seanmalbert left a comment

Choose a reason for hiding this comment

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

lgmt

@seanmalbert seanmalbert merged commit 0565fa4 into dev May 19, 2022
@seanmalbert seanmalbert deleted the 1205/new-listings-views branch May 19, 2022 13:13
seanmalbert added a commit that referenced this pull request Jun 23, 2022
* feat: creating views for public and partner lists

* fix: re-adding the features per sean

* fix: fixes accessibilityFeaturesExist

* fix: updates per sean

* feat: update default listing view

Co-authored-by: Sean Albert <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants