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

Use activated and deactivated lifecycle hooks to handle HTML scroll changes for FullscreenModal component #1978

Closed
rtibbles opened this issue Jul 1, 2020 · 21 comments
Assignees
Labels
DEV: frontend good first issue Self-contained, straightforward, low-complexity help wanted Open source contributors welcome P3 - low Priority: Stretch goal

Comments

@rtibbles
Copy link
Member

rtibbles commented Jul 1, 2020

Summary

Because of a flaw in the Vuetify dialog implementation, when opening a full screen modal, you can get double scroll bars occurring, we currently handle this by disabling scrolling on HTML on mount, but this does not catch all uses cases, particularly when the full screen modal is used in a route and kept alive.

See here for more context: #1962 (comment)

Category

Select one: BUG

@rtibbles rtibbles added this to the Vue Refactor milestone Jul 1, 2020
@rtibbles rtibbles mentioned this issue Jul 1, 2020
10 tasks
@jayoshih jayoshih added the P3 - low Priority: Stretch goal label Aug 3, 2020
@rtibbles rtibbles removed this from the Vue Refactor milestone Jan 19, 2021
@rtibbles rtibbles added DEV: frontend good first issue Self-contained, straightforward, low-complexity and removed frontend labels Mar 12, 2021
@MisRob MisRob added the help wanted Open source contributors welcome label Aug 18, 2023
@Twix03
Copy link

Twix03 commented Dec 19, 2023

I would like to take this issue, can you please help me with how to get started.

@MisRob
Copy link
Member

MisRob commented Jan 9, 2024

Hi @Twix03, thank you for volunteering! I'm sorry for delayed response, the whole team had holidays recently. To get started, please follow Studio Contributing Guidelines, where you will find how to run Studio locally. After your Studio is running, you can reproduce the issue described here. Let us know if you still wish to be assigned.

@Divyansh044
Copy link

@MisRob Please assign this issue to me I can work on this

@MisRob
Copy link
Member

MisRob commented Jan 15, 2024

Hi @Divyansh044, thank you for volunteering. I'm assigning you.

@Divyansh044 Divyansh044 removed their assignment Jan 16, 2024
@nick2432
Copy link
Contributor

can i work on this?

@rtibbles
Copy link
Member Author

Hi @nick2432, you're currently assigned to three issues, so let's see those finished up before we assign any more!

@NightFury742
Copy link

Hey @rtibbles I would be delighted to work on this issue. Can you please assign it to me

@nick2432
Copy link
Contributor

Hi @nick2432, you're currently assigned to three issues, so let's see those finished up before we assign any more!

@rtibbles , yes, I noticed I have two assigned issues. Thank you for your attention, and I apologize for any inconvenience caused.

@MisRob
Copy link
Member

MisRob commented Jan 23, 2024

No problem, @nick2432

@MisRob
Copy link
Member

MisRob commented Jan 23, 2024

Thanks for volunteering, @NightFury742. I will assign you.

@NightFury742
Copy link

@MisRob @rtibbles I can't recreate this issue, can I get some more insights on this ? Also i'm really sorry for the delayed response, I was busy with some personal work.

@rtibbles
Copy link
Member Author

At its root, we should be using the activated and deactivated lifecycle hooks to make the updates to HTML scroll changes, rather than created and destroyed hooks - it's possible that this issue may not be as prevalent now, if we are no longer keeping components alive, so that may be why you are having more difficulty seeing problematic behaviour in action.

@MisRob
Copy link
Member

MisRob commented Mar 15, 2024

Hi @NightFury742, is this issue work in progress or should we unassign?

@NightFury742
Copy link

Really sorry @MisRob for taking so long. I'll unassign it.

@NightFury742 NightFury742 removed their assignment Mar 16, 2024
@MisRob
Copy link
Member

MisRob commented Mar 18, 2024

No problem, thanks for letting us know @NightFury742

@Abhishekzod007
Copy link
Contributor

Hi @MisRob
I think I'll be taking up this issue now
I think an example of this would be the "Edit Channel" modal, please let me know if this is correct

@Abhishekzod007
Copy link
Contributor

Wat I understood from the above conversation is that the mount and destroy based events are proving to be problematic and we should use activated and deactivated lifecycle hooks instead
So, if that's correct. would something like this :


    deavtivated() {
      this.hideHTMLScroll(false); // Ensure scroll is restored when the component is destroyed
    },
    activated(){
      this.hideHTMLScroll(true);
    },
    mounted() {
      this.$refs.dialog.initDetach();
    },

get the job done ?

@rtibbles
Copy link
Member Author

That seems correct, yes.

@rtibbles
Copy link
Member Author

I have assigned you to the issue.

@Abhishekzod007
Copy link
Contributor

That seems correct, yes.

Thanks for affirming, I will draft a PR with the basic changes I mentioned above.
Then we can see if the issue is prevalent in any other scenarios as well.

@MisRob
Copy link
Member

MisRob commented Jul 10, 2024

Closed by #4482

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DEV: frontend good first issue Self-contained, straightforward, low-complexity help wanted Open source contributors welcome P3 - low Priority: Stretch goal
Projects
None yet
Development

No branches or pull requests

9 participants