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: add stops list in the line profile page #929

Merged
merged 4 commits into from
Nov 27, 2024

Conversation

Tami-Co
Copy link
Collaborator

@Tami-Co Tami-Co commented Nov 16, 2024

I added code to display the list of stops for the line.
As a result, there were redundant code sections, so I removed them.

@Tami-Co Tami-Co requested a review from NoamGaash as a code owner November 16, 2024 17:20
@Tami-Co Tami-Co self-assigned this Nov 16, 2024
@NoamGaash
Copy link
Member

Hi @Tami-Co , would you like any help with the lint command?

@Tami-Co
Copy link
Collaborator Author

Tami-Co commented Nov 24, 2024

Hi @Tami-Co , would you like any help with the lint command?

@NoamGaash I would be happy to know how to solve the CI/CD issues that have arisen.
Thanks!

@NoamGaash
Copy link
Member

regarding the lint - running the command npm run lint:fix should solve the problem (use git to commit the lint fixes this command will make)
regarding the title of the pull request - let's add a feat: prefix so it will match conventional commits guidelines.
we have this document with more information if you find it interesting -
https://github.com/hasadna/open-bus-map-search/blob/main/CONTRIBUTING.md#why-do-i-get-a-red-x-commit-status
let me know how can we improve so it will be more accessible

@NoamGaash NoamGaash changed the title Adding the list of stops in the line profile page feat: add stops list in the line profile page Nov 25, 2024
…into Adding_the_list_of_stops_in_the_line_profile_page
@Tami-Co
Copy link
Collaborator Author

Tami-Co commented Nov 25, 2024

All checks have passed.
I would appreciate it if you could review the code.

@NoamGaash
Copy link
Member

Thank you!
I'm not sure I fully understand the feature you have implemented - I can see you have added a dropdown to select a bus stop, but what happen if a user selects one of the stops? Also, if possible, would you provide a screenshot?
I've tried using the preview link to see the dropdown in action (/profile/2419581) but seems like something in the preview is broken:
image

@Tami-Co
Copy link
Collaborator Author

Tami-Co commented Nov 27, 2024

Currently, as a first step, I have added the list of bus stops, as you mentioned here:
צילום מסך 2024-11-27 020335

Therefore, at the moment, when a user selects a bus stop, nothing happens.
Attached is a screenshot as requested:

image

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.

Thank you! I guess I had internet connectivity problem or something as I wasn't able to see it locally. By list I thought of some UL and LI tags, but your solution is great.

@Tami-Co Tami-Co merged commit af09276 into main Nov 27, 2024
16 checks passed
@Tami-Co Tami-Co deleted the Adding_the_list_of_stops_in_the_line_profile_page branch November 27, 2024 10:04
@NoamGaash NoamGaash linked an issue Nov 27, 2024 that may be closed by this pull request
@NoamGaash
Copy link
Member

@all-contributors could you please add @Tami-Co as a code contributor?

Copy link
Contributor

@NoamGaash

I've put up a pull request to add @Tami-Co! 🎉

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.

Adding the list of stops in the line profile page
2 participants