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

StatusCard on mobile screens hides the device on the map #1155

Merged
merged 3 commits into from
Jul 29, 2023
Merged

StatusCard on mobile screens hides the device on the map #1155

merged 3 commits into from
Jul 29, 2023

Conversation

jcardus
Copy link
Contributor

@jcardus jcardus commented Jul 29, 2023

before:
traccar fleetmap io_(iPhone SE) (1)

after:
localhost_3000_(iPhone SE)

@@ -57,6 +57,8 @@ const useStyles = makeStyles((theme) => ({
content: {
paddingTop: theme.spacing(1),
paddingBottom: theme.spacing(1),
'max-height': '40vh',
overflow: 'hidden',
Copy link
Member

Choose a reason for hiding this comment

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

Why are we hiding overflow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because it overlaps with the actions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

without overflow: hidden

localhost_3000_(iPhone SE) (3)

Copy link
Member

Choose a reason for hiding this comment

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

Screenshot please. And also wouldn't it mean that it's not the content is not accessible?

Copy link
Contributor Author

@jcardus jcardus Jul 29, 2023

Choose a reason for hiding this comment

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

yes, the overflow content won't be accesible

Copy link
Member

Choose a reason for hiding this comment

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

That's not an acceptable solution then. It should become scrollable if you want to limit the height.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to overflow: scroll
localhost_3000_(iPhone SE) (4)

@@ -57,6 +57,8 @@ const useStyles = makeStyles((theme) => ({
content: {
paddingTop: theme.spacing(1),
paddingBottom: theme.spacing(1),
'max-height': '40vh',
Copy link
Member

Choose a reason for hiding this comment

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

You have to use camelcase name here. And also the constant should be in the same place where we put other similar constants. It's part of the theme.

@jcardus
Copy link
Contributor Author

jcardus commented Jul 29, 2023

please check

@tananaev tananaev merged commit f5a4d73 into traccar:master Jul 29, 2023
1 check passed
@tananaev
Copy link
Member

Merged, thanks.

@jinzo jinzo mentioned this pull request Jul 30, 2023
@tananaev
Copy link
Member

@jcardus is looks like the card is broken by this change. Tested on Chrome on Mac. Can you please fix.

Screenshot 2023-08-19 at 2 05 04 PM

@cop98 cop98 mentioned this pull request Sep 6, 2023
@jcardus
Copy link
Contributor Author

jcardus commented Sep 12, 2023

Sorry @tananaev, I just saw this now. Is this still an issue?

@tananaev
Copy link
Member

I forgot. Maybe I already fixed it?

@jcardus
Copy link
Contributor Author

jcardus commented Sep 12, 2023

I can't reproduce it.

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.

None yet

2 participants