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.

Sign up for GitHub

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

Jump to bottom

[feature] Turbo Drive and Sessions in iOS #1542

Merged
merged 12 commits into from
Apr 29, 2023
Merged

[feature] Turbo Drive and Sessions in iOS #1542

merged 12 commits into from
Apr 29, 2023

Conversation

Pralish
Copy link
Contributor

@Pralish Pralish commented Apr 22, 2023 •
edited by donrestarone
Loading

Addresses: #1414 & #820

@github-actions
Copy link

Deployed review-app can be viewed at https://review-1542.violet-test.net

@donrestarone
Copy link
Contributor

donrestarone commented Apr 23, 2023 •
edited
Loading

@Pralish for testing, I cloned the iOS repo: https://github.com/restarone/violet_rails_ios_client (built the project in xcode) and pointed the app root to your review app here: https://github.com/restarone/violet_rails_ios_client/blob/main/Violet%20Rails/Common/Constants/App.swift

This is definitely an improvement, and I think you can use this technique to debug the server/client faster
Screen Shot 2023-04-23 at 8 54 55 AM

EDIT: issue is there in web as well. So Im assuming the root cause has been identified and the UI just needs to be fixed.

Screen Shot 2023-04-23 at 8 57 39 AM

@github-actions
Copy link

Deployed review-app can be viewed at https://review-1542.violet-test.net

@donrestarone
Copy link
Contributor

feedback:

logging in works, but the following links are broken:

IMG_2760

@github-actions
Copy link

Deployed review-app can be viewed at https://review-1542.violet-test.net

@donrestarone
Copy link
Contributor

donrestarone commented Apr 24, 2023 •
edited
Loading

test plan 🧪

  1. HTML Renderer - WITHOUT TURBO videos are playable on first visit ✅
  2. HTML Renderer - WITHOUT TURBO videos playtime is tracked on first visit ✅
  3. HTML Renderer - WITHOUT TURBO videos playtime is tracked on user-bounce ✅
  4. HTML Renderer - WITH TURBO videos are playable on first visit ❌
  5. HTML Renderer - WITH TURBO videos playtime is tracked on first visit ⌛
  6. HTML Renderer - WITH TURBO videos playtime is tracked on user-bounce ⌛
  7. Password reset ✅
  8. Account creation - with 2FA ✅
  9. Account creation - without 2FA ✅
  10. Account creation by invite - with 2FA ✅
  11. Account creation by invite - without 2FA ✅
  12. Sign in - with 2FA ✅
  13. Sign in - without 2FA ✅
  14. Sign out ✅
  15. API Form redirect flash message ✅
  16. Error flash messages ✅
  17. Success flash messages ✅
  18. sign in redirect (redirected to desired path after sign in) ✅
  19. Temporary deployment to nikean.org ✅
  20. Viewing an API Namespace ✅
  21. Editing an API Namespace ✅

@github-actions
Copy link

Deployed review-app can be viewed at https://review-1542.violet-test.net

@github-actions
Copy link

Deployed review-app can be viewed at https://review-1542.violet-test.net

@github-actions
Copy link

Deployed review-app can be viewed at https://review-1542.violet-test.net

@github-actions
Copy link

Deployed review-app can be viewed at https://review-1542.violet-test.net

@github-actions
Copy link

Deployed review-app can be viewed at https://review-1542.violet-test.net

@github-actions
Copy link

Deployed review-app can be viewed at https://review-1542.violet-test.net

@donrestarone
Copy link
Contributor

known issues: flash messages are not working on iOS, and

If we want to enable turbo for the whole app, we need a feature that was introduced in turbo 7.2.0, which is not compatible with node 10

hotwired/turbo#445

Or when we upgrade to rails 7

@Pralish Pralish marked this pull request as ready for review April 25, 2023 13:50
@Pralish Pralish changed the title [WIP] turbo in signin turbo in signin Apr 25, 2023
@donrestarone
Copy link
Contributor

donrestarone commented Apr 25, 2023 •
edited
Loading

@Pralish videos don't play on turbo navigation, may or may not need a fix ATM but capturing for future:

when turbo enabled
Screen Shot 2023-04-25 at 9 57 03 AM
it does not show play button:
Screen Shot 2023-04-25 at 9 57 17 AM

@donrestarone donrestarone changed the title turbo in signin [feature] Turbo Drive and Sessions in iOS Apr 25, 2023
@donrestarone donrestarone added the Pending UAT on Testnet ⚠️ currently on violet-test.net and being tested label Apr 26, 2023
@github-actions
Copy link

Deployed review-app can be viewed at https://review-1542.violet-test.net

1 similar comment
@github-actions
Copy link

Deployed review-app can be viewed at https://review-1542.violet-test.net

donrestarone
donrestarone reviewed Apr 27, 2023
View reviewed changes
app/views/users/shared/_links.html.erb
@@ -2,7 +2,7 @@
<%= link_to "Log in", new_session_path(resource_name) %><br />
<% end %>

<%- if devise_mapping.registerable? && controller_name != 'registrations' %>
<%- if devise_mapping.registerable? && controller_name != 'registrations' && Subdomain.current.allow_user_self_signup %>
Copy link
Contributor

Choose a reason for hiding this comment

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

nice touch

@donrestarone
Copy link
Contributor

@Pralish when 2FA is turned off, sign-in does not work
Screen Shot 2023-04-27 at 9 45 02 AM

RPReplay_Final1682603079.MP4

This issue will need to be fixed

@donrestarone
Copy link
Contributor

@Pralish on first render cookie consent form is broken, this will need to be fixed:
Screen Shot 2023-04-27 at 9 47 42 AM

@donrestarone donrestarone added Failing UAT ❌ ☠️ and removed Pending UAT on Testnet ⚠️ currently on violet-test.net and being tested labels Apr 27, 2023
@github-actions
Copy link

Deployed review-app can be viewed at https://review-1542.violet-test.net

@github-actions
Copy link

Deployed review-app can be viewed at https://review-1542.violet-test.net

@Pralish
Copy link
Contributor Author

Pralish commented Apr 27, 2023

Had to rename views with only .haml to .html.haml for redirect with turbo to work properly.

Ref:
hotwired/turbo#138 (comment)

@Pralish
Copy link
Contributor Author

Pralish commented Apr 28, 2023

@donrestarone , the issue has been fixed

@donrestarone donrestarone added Pending UAT on Testnet ⚠️ currently on violet-test.net and being tested and removed Failing UAT ❌ ☠️ labels Apr 28, 2023
@github-actions
Copy link

Deployed review-app can be viewed at https://review-1542.violet-test.net

@donrestarone
Copy link
Contributor

⚠️ temporary deployment to nikean.org

@donrestarone donrestarone changed the base branch from master to rc April 29, 2023 01:00
@donrestarone donrestarone merged commit 86db27d into rc Apr 29, 2023
donrestarone added a commit that referenced this pull request Apr 29, 2023
@donrestarone @Pralish
[feature] Turbo Drive and Sessions in iOS (#1542) (#1547)
2e684c6 
Addresses: #1414 & #820

Addresses: #1414 & #820

## Allow login on iOS

<img width="503" alt="Screen Shot 2023-04-28 at 10 38 00 PM" src="https://user-images.githubusercontent.com/35935196/235279807-f985c695-07d4-40b5-a8dc-26aa7e045286.png">

## Access Forum, Web app and Admin Index 
<img width="496" alt="Screen Shot 2023-04-28 at 10 37 39 PM" src="https://user-images.githubusercontent.com/35935196/235279808-7d061262-318a-408c-8eec-896365d69359.png">



Co-authored-by: Pralish Kayastha <50227291+Pralish@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@donrestarone donrestarone donrestarone left review comments

Assignees
No one assigned
Labels
deploy-review-app Pending UAT on Testnet ⚠️ currently on violet-test.net and being tested
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Pralish @donrestarone