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

Dt 5248 Display subway entrance information #5235

Merged
merged 8 commits into from
Feb 17, 2025
Merged

Dt 5248 Display subway entrance information #5235

merged 8 commits into from
Feb 17, 2025

Conversation

HenrikSundell
Copy link
Contributor

Proposed Changes

Pull Request Check List

  • A reasonable set of unit tests is included
  • Console does not show new warnings/errors
  • Changes are documented or they are self explanatory
  • This pull request does not have any merge conflicts
  • All existing tests pass in CI build

Review

  • Read and verify the code changes
  • Test the functionality by running the UI locally with all popular browsers available in your platform
  • Check that the implementation matches the design, when such one is defined in a Jira issue
  • Merge the pull request

Copy link
Member

@vesameskanen vesameskanen left a comment

Choose a reason for hiding this comment

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

I tested the PR wih HSL ui theme. The only difference I could see is an additional itinerary detail row 'Sisäänkäynti M / Uloskäynti M', which is quite useless information.

Is this a data issue? Where can I find more accurate entrance labels?

Also, there seems to be two PRs open, one is a draft and apparenly has wrong title. Please clean up.

@HenrikSundell HenrikSundell changed the title Dt 5248 Dt 5248 Display subway entrance information Feb 4, 2025
Copy link
Member

@vesameskanen vesameskanen left a comment

Choose a reason for hiding this comment

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

Some review notes:

  • Steissi - Itis metro journey renders two entrances A and D on map in Itis. Is this intentional?
  • New figma design shows map icons vertically - not horizontally - aligned. There is no wheelchair icon on map, it is only shown in a map popup which is missing now. Are these differences intentional?
  • Figma designs shows a narrow gap between entrance icons. Currently there is no separating space.
  • Icons always render quite large. Other map icons scale by zoom level. Is this by design?

@vesameskanen vesameskanen merged commit 04f81d6 into v3 Feb 17, 2025
6 checks passed
@vesameskanen vesameskanen deleted the DT-5248 branch February 17, 2025 08:31
vesameskanen added a commit that referenced this pull request Mar 4, 2025
This reverts commit 04f81d6, reversing
changes made to 7042bf5.
vesameskanen added a commit that referenced this pull request Mar 5, 2025
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.

2 participants