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: Added complaint button, hidden as an easter egg 'complaint'. #791

Merged
merged 13 commits into from
Jun 18, 2024

Conversation

LiadOvdat5
Copy link
Contributor

@LiadOvdat5 LiadOvdat5 commented Jun 9, 2024

New Feature

Description

When opened you can write your info and submit.
to show the button, write 'complaint'

screenshots

image
image

when opened you can write your info and submit.
@LiadOvdat5 LiadOvdat5 changed the title Added complaint button, hidden as an easter egg 'complaint'. A bug fix: Added complaint button, hidden as an easter egg 'complaint'. Jun 9, 2024
@LiadOvdat5 LiadOvdat5 changed the title A bug fix: Added complaint button, hidden as an easter egg 'complaint'. fix: Added complaint button, hidden as an easter egg 'complaint'. Jun 9, 2024
@LiadOvdat5 LiadOvdat5 changed the title fix: Added complaint button, hidden as an easter egg 'complaint'. feat: Added complaint button, hidden as an easter egg 'complaint'. Jun 9, 2024
Copy link
Member

@NoamGaash NoamGaash left a comment

Choose a reason for hiding this comment

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

good job! do you need any help with the linting warnings?

@LiadOvdat5
Copy link
Contributor Author

LiadOvdat5 commented Jun 11, 2024

good job! do you need any help with the linting warnings?

Yes I would love that,
I have warning on the any in siriRide in the interface

image

what should I put there instead?
Or I should just import them as in the BusToolTip (using useEffect at the start)?
image

@NoamGaash NoamGaash self-requested a review June 13, 2024 13:13
@supervxn
Copy link
Collaborator

@LiadOvdat5 - the frontend side looks clear and simple. 😺.

Copy link
Member

@NoamGaash NoamGaash left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, seems like you solved all linting issues + the code seems great!
We would love to have this merged. Thank you! 👏 🥇

Copy link
Member

@NoamGaash NoamGaash left a comment

Choose a reason for hiding this comment

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

some comments - can be fixed after merge, or be opened as a new issue

Comment on lines +125 to +132
<TextField
label={t('email')}
name="email"
value={complaintData.email}
onChange={handleInputChange}
fullWidth
margin="normal"
/>
Copy link
Member

Choose a reason for hiding this comment

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

if you'll add type=email, then it will show better keyboard on some devices and also be more accessible.
same can be done for numbers and phone numbers
(can be done in future PR)

@@ -134,6 +134,29 @@
"coords": "נ.צ.",
"hide_document": "הסתר מידע לגיקים",
"show_document": "הצג מידע לגיקים",
"open_complaint": "פתח תלונה",
"close_complaint": "סגור תלונה",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"close_complaint": "סגור תלונה",
"close_complaint": "בטל תלונה",

</form>
<Button
variant="contained"
color="success"
Copy link
Member

Choose a reason for hiding this comment

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

this button will lead the user to cancel their complaint and potentially loose everything they typed

Suggested change
color="success"
color="danger"

border: '2px solid #000',
boxShadow: 24,
p: 4,
textAlign: i18n.language === 'he' ? 'left' : 'right',
Copy link
Member

Choose a reason for hiding this comment

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

something feels weird about this condition, are you sure it works as intended?

@supervxn
Copy link
Collaborator

Loving the speed here 🙌.
Let me know when it's live.

@NoamGaash
Copy link
Member

NoamGaash commented Jun 14, 2024

@supervxn Once the pull request is merged (you'll probably get a notification about it) the deployment process will start automatically. It takes 10 minutes on average, but sometime (rarely) it gets stuck for about a day

Copy link
Contributor

github-actions bot commented Jun 18, 2024

@NoamGaash
Copy link
Member

@supervxn
From now on previews are also available on forks, so you can see the feature live.

@NoamGaash NoamGaash merged commit dbbf9a0 into hasadna:main Jun 18, 2024
13 checks passed
@NoamGaash
Copy link
Member

@all-contributors please add @LiadOvdat5 for code

Copy link
Contributor

@NoamGaash

I've put up a pull request to add @LiadOvdat5! 🎉

@supervxn
Copy link
Collaborator

supervxn commented Jun 18, 2024

@supervxn From now on previews are also available on forks, so you can see the feature live.

is this the right fork?

If so, how can I preview (without installing anything)?
https://github.com/LiadOvdat5/open-bus-map-search

@NoamGaash
Copy link
Member

@supervxn have you seen this comment?
image
#791 (comment)

@supervxn
Copy link
Collaborator

@NoamGaash
Copy link
Member

did you click on a bus icon inside the real-time map page before typing complaint?
did you use english keyboard?

@NoamGaash
Copy link
Member

Also, now when this PR is merged, you can see it on production
https://open-bus-map-search.hasadna.org.il/map

@supervxn
Copy link
Collaborator

supervxn commented Jun 18, 2024

Got it! Thanks @LiadOvdat5 - you rock!

BTW: is the form connected to somewhere already?

@NoamGaash
Copy link
Member

not yet

@supervxn
Copy link
Collaborator

not yet

Phew.

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.

3 participants