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

revert-breaking-changes #1511

Closed
wants to merge 2 commits into from

Conversation

vishalvivekm
Copy link
Contributor

@vishalvivekm vishalvivekm commented Dec 6, 2023

Description

Meshery.io/catalog is crashing in iOS safari and the first PR that seems to be having this turns out to be : #1186
This PR reverts changes done in 1186 to no avail.

screencast from BroswerStack iphone 12 pro:

iPhone.12.Pro.v14.4.-.Google.Chrome.2023-12-07.12-41-02.mp4

Notes for Reviewers

Signed commits

  • Yes, I signed my commits.

Signed-off-by: vishalvivekm <[email protected]>
@vishalvivekm vishalvivekm added pr/do-not-merge PRs not ready to be merged pr/draft WIP/Draft pull request labels Dec 6, 2023
Copy link

netlify bot commented Dec 6, 2023

Deploy Preview for mesheryio-preview ready!

Name Link
🔨 Latest commit e82fe70
🔍 Latest deploy log https://app.netlify.com/sites/mesheryio-preview/deploys/6571b0534775a400089c7e3d
😎 Deploy Preview https://deploy-preview-1511--mesheryio-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@vishalvivekm
Copy link
Contributor Author

vishalvivekm commented Dec 7, 2023

@iArchitSharma
Copy link
Contributor

@vishalvivekm its still breaking

@GaganpreetKaurKalsi
Copy link
Contributor

The page loads and reloads and then crashes

@iArchitSharma
Copy link
Contributor

Also, this is how it looks in IOS device before this PR i.e #1185 was merged
A372128D-454E-4747-9E1C-C455DD83AEC8.pdf

@vishalvivekm
Copy link
Contributor Author

Also, this is how it looks in IOS device before this PR i.e #1185 was merged A372128D-454E-4747-9E1C-C455DD83AEC8.pdf

Thanks for confirming @GaganpreetKaurKalsi @iArchitSharma, we now know that we hafta start with the #1186 and that it was indeed the first pr with crashing catalog page.

@Savio629
Copy link
Contributor

Savio629 commented Dec 7, 2023

The pr #1186 included Visibility of cards for All option on mobile and a View All button.
Might be because of Js/css code.
It's difficult to check without a IOS device.
Will update if I get the solution.

One of the solution I got for this is error:
For IOS we can't keep display:none, we need to have some other alternative to hide the other content it.

Signed-off-by: vishalvivekm <[email protected]>
@leecalcote
Copy link
Member

Is this PR needed any longer?

@vishalvivekm
Copy link
Contributor Author

Is this PR needed any longer?

It's not. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/catalog area/website pr/do-not-merge PRs not ready to be merged pr/draft WIP/Draft pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants