-
Notifications
You must be signed in to change notification settings - Fork 22
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
1267 remove feedback feature #1348
Conversation
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.
Thanks for submitting this! There are a few more places where you need to remove references to the feedback feature, but this otherwise looks great to me!
@@ -60,9 +58,6 @@ export const OrganizationListingPage = () => { | |||
); |
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.
On the lines just above this, could you remove the "feedback"
string from both arrays? I can't comment directly on the lines, since they're just beyond the range of what the GitHub interface allows me to comment on, but they're just above this line.
@@ -66,9 +64,6 @@ export const ServiceListingPage = () => { | |||
); |
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.
Similar comment about removing "feedback"
on the lines above this.
className="feedback__Modal" | ||
overlayClassName="feedback__Overlay" |
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.
These are CSS classes that are actually still defined in their corresponding CSS file. Could you remove the following lines here?
askdarcel-web/app/components/listing/ActionBar.scss
Lines 39 to 71 in f6db93a
.feedback__Overlay { | |
position: fixed; | |
top: 0; | |
left: 0; | |
right: 0; | |
bottom: 0; | |
background-color: rgba(94, 94, 94, 0.6); | |
overflow: auto; | |
} | |
.feedback__Modal { | |
position: absolute; | |
margin: 0 auto; | |
top: 20%; | |
left: 0; | |
right: 0; | |
width: 570px; | |
padding: 34px 46px; | |
background-color: $color-white; | |
box-shadow: 0 3px 6px rgba(0, 0, 0, 0.25); | |
border-radius: 10px; | |
@media screen and (max-width: 767px) { | |
top: 5%; | |
width: 100%; | |
height: 95%; | |
padding: 40px 25px; | |
} | |
} | |
.feedback-modal { | |
display: none; | |
} |
…Page.tsx abd ServiceListingPage.tsx. Remove feedback form related classnames from ActionBar.scss.
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.
Thanks for making this change! This PR LGTM now. Our usual process is that once a PR has been approved, the submitter is the one who hits the merge button, so feel free to do that at your leisure.
Richard,
Hitting that merge button felt good! Looking forward to many more hits or
buttons :) whichever it may be!
Thank you!
…On Tue, May 14, 2024 at 1:57 PM Richard Xia ***@***.***> wrote:
***@***.**** approved this pull request.
Thanks for making this change! This PR LGTM now. Our usual process is that
once a PR has been approved, the submitter is the one who hits the merge
button, so feel free to do that at your leisure.
—
Reply to this email directly, view it on GitHub
<#1348 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKJ4DB3PLCGWVBLD4EPMIVTZCJUBJAVCNFSM6AAAAABHTHL6R6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANJWGI4DMMJSGM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Hi Richard,
Should I start working on any other ticket?
Thanks!
On Tue, May 14, 2024 at 2:06 PM Adlan Yandarbiev ***@***.***>
wrote:
… Richard,
Hitting that merge button felt good! Looking forward to many more hits or
buttons :) whichever it may be!
Thank you!
On Tue, May 14, 2024 at 1:57 PM Richard Xia ***@***.***>
wrote:
> ***@***.**** approved this pull request.
>
> Thanks for making this change! This PR LGTM now. Our usual process is
> that once a PR has been approved, the submitter is the one who hits the
> merge button, so feel free to do that at your leisure.
>
> —
> Reply to this email directly, view it on GitHub
> <#1348 (review)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AKJ4DB3PLCGWVBLD4EPMIVTZCJUBJAVCNFSM6AAAAABHTHL6R6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANJWGI4DMMJSGM>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
|
Thanks for your contribution, Adlan! I think this ticket would be a good one for you to tackle: #1313 |
Brian,
Looking good!
Thank you!
…On Fri, May 17, 2024 at 11:17 AM Brian Schroer ***@***.***> wrote:
Thanks for your contribution, Adlan! I think this ticket would be a good
one for you to tackle: #1313
<#1313>
—
Reply to this email directly, view it on GitHub
<#1348 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKJ4DB2PVE7XQWX5ZWE2NXDZCY3RBAVCNFSM6AAAAABHTHL6R6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJYGA2DCNZTGE>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
No description provided.