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

Click on map full screen triggers minimization of mobile browser tab bar #835

Closed
2 tasks done
andrewtavis opened this issue Apr 22, 2024 · 18 comments
Closed
2 tasks done
Assignees
Labels
feature New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@andrewtavis
Copy link
Member

andrewtavis commented Apr 22, 2024

Terms

Description

The MediaMap component can be seen on https://activist.org/events/ANY_INTEGER/about. One thing that would be a nice feature would be to make sure that the browser tab bar Safari, Chrome and other browsers would retract/minimize so that the map is the full screen and we don't have the browser search bar as well. An example with the search bar still visible at the bottom of the screen is:

IMG_0172

The documentation for the FullscreenControl class can be seen here. By the looks of it there's a fullscreenstart event that could be used to potentially trigger the search/tab bar being minimized.

Contribution

Would be happy to support on this or get to it myself later 😊 This would be a great feature to add into the already strong map component, as the user experience is a bit reduced when the browser controls are visible on full screen.

@andrewtavis andrewtavis added feature New feature or request help wanted Extra attention is needed good first issue Good for newcomers labels Apr 22, 2024
@Geet-S
Copy link

Geet-S commented Jun 18, 2024

hey, still need help with this?
From what I've checked on PC and android with Chrome browser this doesn't seem to be a problem but it could be different on Safari

@andrewtavis
Copy link
Member Author

Would be great to get some help with this, @Geet-S! Let us know what your thoughts are, or also feel free to send along a PR :) Happy to help as needed 😊

@andrewtavis
Copy link
Member Author

Hey hey @Geet-S 👋 Checking in to see if there's anything that we can do to assist here :)

@Geet-S
Copy link

Geet-S commented Jul 2, 2024

Hey sorry, I weirdly never got notified on your reply to me I don't know what's up with that.

So, I don't really have any apple products to check on safari if it's in full screen or not like you described, but let me know if there is a way for me to test that out through some emulator of sorts, I'm kinda new to this :P I'll try to figure it out from there

@andrewtavis
Copy link
Member Author

Hey @Geet-S 👋 No stress at all. You'd be welcome to send along something and I could test it on my end :) Seems like the following pages might have suggestions we could try:

Looks like we could scroll to the top to trigger it hiding, which could work as discussed in the SO question, or there's also some CSS and JS for the viewport in the first link. Which do you think would make sense?

@Geet-S
Copy link

Geet-S commented Jul 7, 2024

Some sources seem to say the scroll method doesn't work after iOS7 as apple 'fixed' it
Looks like the JS and CSS method might be our best bet at the moment

@andrewtavis
Copy link
Member Author

Sounds good, thanks for checking! Do you want to give it a shot as best as the research says, and then I can test it on the PR?

@Geet-S
Copy link

Geet-S commented Jul 10, 2024

yeah, sure, I am quite new to working on open source so I do have a few clarifications
I'll probably have to make changes in frontend/components/media/MediaMap.vue and frontend/assets/tailwind.css, yes? Correct me if I'm wrong here
And considering the example in the website is given in plain js and css I'll probably have to convert the code to vue and tailwind, right?
(I promise I know how to code :P I just like to be sure before going in)

@andrewtavis
Copy link
Member Author

This all sounds great, @Geet-S! You might need to use plan CSS instead of Tailwind, which can be done within a <style> block for Vue. I'd say do your best and also list out a couple of options to try if it's not working in the PR. From there I can use the deployment preview on my device to see if it's working, and can make any edits that are needed to get it working.

Once this is done we can shift to an issue that's a bit easier for you to test yourself, if you'd be interested 😊

@Geet-S
Copy link

Geet-S commented Jul 10, 2024

Alright thanks for clarifying! You don't have to answer this question but I'd like to just learn: is it not considered bad practice to mix vanilla CSS and tailwind?

Also I'd love to take up another issue after this! I've been trying to get into open source and put it off for a long time because it seemed very daunting, I really appreciate you willing to let me find my way around here :)

@andrewtavis
Copy link
Member Author

Happy to help, and thanks for your dedication here! Re your question: very valid :) We've found that in some cases we do need to use style blocks when a specific page element needs to be targeted, say within a third party component like styling for the captcha component we use. Aside from that, yes we should definitely be using Tailwind wherever possible 😊

@Geet-S
Copy link

Geet-S commented Jul 10, 2024

Alright, got it, thank you for the knowledge!
I'll update you soon on how it's going with the issue

@andrewtavis
Copy link
Member Author

Happy to share, and looking forward to the PR! :)

@Geet-S
Copy link

Geet-S commented Jul 12, 2024

Hey @andrewtavis
So as I understand, we use a GL for the map and it has a built-in control to trigger full-screen. We probably have to remove that and make a custom control for it to trigger the js and css code we found, right?

@andrewtavis
Copy link
Member Author

Maybe we can trigger that based on a reference of the control? If memory serves me they all have ids, so we add an extra click action?

@Geet-S
Copy link

Geet-S commented Jul 13, 2024

Ah alright I'll try that, thanks

@andrewtavis
Copy link
Member Author

Very welcome, @Geet-S! Thanks for discussing this :) Definitely makes sense that it's a bit more involved.

@andrewtavis
Copy link
Member Author

I just tried a ton of combinations here in #979, and none of the suggestions that I've found have worked for this. Beyond what's been tried, I saw multiple times that this isn't possible, so I think that it's safe to close this and focus elsewhere :)

Thanks for the conversation here, @Geet-S!

@github-project-automation github-project-automation bot moved this from In Progress to Done in activist Board Sep 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
Archived in project
Development

No branches or pull requests

2 participants