-
Notifications
You must be signed in to change notification settings - Fork 24
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
1427/listing management preferences #1564
Conversation
✔️ Deploy Preview for dev-bloom ready! 🔨 Explore the source changes: 5badfd2 🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-bloom/deploys/6101953bfa92b20008fb179c 😎 Browse the preview: https://deploy-preview-1564--dev-bloom.netlify.app |
✔️ Deploy Preview for jovial-davinci-1d67a0 ready! 🔨 Explore the source changes: 5badfd2 🔍 Inspect the deploy log: https://app.netlify.com/sites/jovial-davinci-1d67a0/deploys/6101953bdc38720007e37502 😎 Browse the preview: https://deploy-preview-1564--jovial-davinci-1d67a0.netlify.app |
✔️ Deploy Preview for dev-partners-bloom ready! 🔨 Explore the source changes: 5badfd2 🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-partners-bloom/deploys/6101953b486c30000726f23a 😎 Browse the preview: https://deploy-preview-1564--dev-partners-bloom.netlify.app |
✔️ Deploy Preview for dev-storybook-bloom ready! 🔨 Explore the source changes: 5badfd2 🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-storybook-bloom/deploys/6101953b8b22e90008939f32 😎 Browse the preview: https://deploy-preview-1564--dev-storybook-bloom.netlify.app |
0deeb03
to
bed0c3a
Compare
@@ -44,7 +44,6 @@ | |||
|
|||
.fixed-overlay__inner { | |||
transition-property: transform, opacity; | |||
transform: translate(0px, 0px); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was causing a visual bug in the draggable table (weird, I know) - the best suggestion from forums on the package was to remove this style. Let me know if this will cause any other issues, but removing it didn't seem to cause any changes at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@emilyjablonski just a few minor comments, really great work!
LGTM
inset | ||
> | ||
<ViewItem label={t("listings.activePreferences")} className={"mb-2"} /> | ||
{listing?.preferences.length ? ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be more clear to check preferenceTableData
length, instead of listing?.preferences.length
because we use this data here :)
@@ -3,6 +3,7 @@ import { TableHeaders, StandardTable } from "./StandardTable" | |||
|
|||
interface MinimalTableProps { | |||
draggable?: boolean | |||
setData?: (data: any[]) => void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think unknown
would be better as a type here.
@@ -26,6 +26,7 @@ export const TableThumbnail = (props: { children: React.ReactNode }) => { | |||
|
|||
export interface StandardTableProps { | |||
draggable?: boolean | |||
setData?: (data: any[]) => void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above.
995976f
to
fc36b2c
Compare
* 1427/listing management preferences * submit and display preferences * changelog * dragging wip * dragging wip 2 * save order bug * cleanup * add protective optionals * pr feedback * adds key to react fragment Co-authored-by: seanmalbert <smabert@gmail.com>
Pull Request Template
Issue
Addresses #1427
Description
Adds the preferences section to listings management. Check it out in partners.
The issue has some designs for separating preferences by jurisdiction but we don't currently have that data so I wasn't able to do that (unclear if that should be handled here or not, pinged the channel and commented on the issue). So for now I'm just filtering on unique preferences. Keep in mind the two housing preferences from Coliseum in our seed data are treated a little differently and purposefully do not appear on the listing page.
Type of change
How Can This Be Tested/Reviewed?
Create a new listing in listings management and check out the new preferences section. Save the listing then re-edit it. Preview the preferences on the listing page.
Checklist:
Note: react-beautiful-dnd is throwing a scroll warning on drag but it doesn't seem to actually be causing any issue, and based on the activity on the issue about it, I'm not sure there is anything I can do to remove the warning until they put in a fix