Skip to content

fix: introduce sticky inputs section #1143

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

Closed
wants to merge 0 commits into from
Closed

Conversation

krasnoff
Copy link
Collaborator

@krasnoff krasnoff commented Jun 4, 2025

Description

I have defined the headers in the time map, single-line-map and single-vehicle-map headers to a css stiky headers

resolving issue: #1059

@krasnoff krasnoff requested a review from NoamGaash as a code owner June 4, 2025 11:45
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.

that's a good start, but I'd imagine it a little differently - when the user starts scrolling down, we can collapse the top navigation bar, hide the title and give more room to the actual map.
what do you think?

src/App.scss Outdated
}

.height-725 {
Copy link
Member

Choose a reason for hiding this comment

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

can we use more meaningful class-names / selectors?
for example, .map-container .position-stiky-header

Comment on lines 82 to 83
<div className="position-stiky-header">
<Typography className="page-title" variant="h4">
Copy link
Member

Choose a reason for hiding this comment

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

why not using the same element?

Suggested change
<div className="position-stiky-header">
<Typography className="page-title" variant="h4">
<Typography className="page-title position-sticky-header" variant="h4">

@krasnoff
Copy link
Collaborator Author

krasnoff commented Jun 5, 2025

data-bus.png

Here is the map page with all it's components
What do you want me to hide when scrolling the map?
Hiding the top navigation bar will be complicated because we will have to change the whole infrastructure.
Hiding the title and the form will be a lot easier.

@krasnoff
Copy link
Collaborator Author

krasnoff commented Jun 5, 2025

Do you have tailwind installed in the app or not?
It can help me to solve some css problems in the app

@NoamGaash
Copy link
Member

I'm not sure if I'll have the capacity, but feel free to bring that to the f2f meeting

@NoamGaash
Copy link
Member

I think hiding the top navigation bar would make sense in all pages.
Hiding the title and making the form smaller (without hiding it completely) makes sense to me.

wdyt?

@krasnoff
Copy link
Collaborator Author

krasnoff commented Jun 5, 2025

I would simply adjust the map height so it will fill the screen without covering the title, navigation bars, etc. There is an option for the map to be displayed as a full page if the user wishes.
I dislike inner scrolling personally. This annoys the user and makes the whole page difficult to use.
It will also a lot easier to implement.

How do you feel about this?

@NoamGaash
Copy link
Member

I don't know - some users have short screens

@NoamGaash
Copy link
Member

also, the fullscreen button is very buggy in most of the screens

@krasnoff krasnoff closed this Jun 8, 2025
@krasnoff krasnoff force-pushed the fix/stiky-input-define branch from 3015772 to 2915a58 Compare June 8, 2025 17:45
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