Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Clean-up the fallback login code #7657

Merged
merged 11 commits into from
Jun 10, 2020
Merged

Clean-up the fallback login code #7657

merged 11 commits into from
Jun 10, 2020

Conversation

clokep
Copy link
Member

@clokep clokep commented Jun 8, 2020 •
edited
Loading

This handles a review comment (#7629 (comment)) and a bunch of other misc. changes to the fallback login code to with an aim to:

  • Be more consistent in styling.
  • Be a bit more modern.

There should be no functional changes in this PR.

I do note that the spec says:

The page will attempt to call the JavaScript function window.onLogin when login has been successfully completed.

But it seems the code calls window.matrixLogin.onLogin unless I'm missing something?

@clokep clokep marked this pull request as ready for review June 9, 2020 17:17
@clokep clokep requested a review from a team June 9, 2020 17:18
@clokep
Copy link
Member Author

clokep commented Jun 9, 2020

This kind of grew a bit, but should be easily reviewable commit by commit. Please let me know if there are any questions!

I made some style changes for consistency, but we can change the style if we want to!

@clokep
Copy link
Member Author

clokep commented Jun 9, 2020

I do note that the spec says:

The page will attempt to call the JavaScript function window.onLogin when login has been successfully completed.

But it seems the code calls window.matrixLogin.onLogin unless I'm missing something?

Filed matrix-org/matrix-doc#2620 to look into this issue.

@richvdh richvdh self-assigned this Jun 10, 2020
richvdh
richvdh approved these changes Jun 10, 2020
View reviewed changes
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm!

@richvdh
Copy link
Member

richvdh commented Jun 10, 2020

we can change the style if we want to!

ftr, if we were after style guides, I'd follow those from the matrix-react-sdk; but I think that what you've done here is pretty consistent with that.

@clokep
Copy link
Member Author

clokep commented Jun 10, 2020

we can change the style if we want to!

ftr, if we were after style guides, I'd follow those from the matrix-react-sdk; but I think that what you've done here is pretty consistent with that.

Seems to follow that, yes. 👍 Thanks!

@clokep clokep merged commit 191dc98 into develop Jun 10, 2020
@clokep clokep deleted the clokep/login-clean-up branch June 10, 2020 13:50
babolivier pushed a commit that referenced this pull request Sep 1, 2021
@anoadragon453
Merge commit '03619324f' into dinsic
70032b3 
* commit '03619324f':
  Create a ListenerConfig object (#7681)
  Fix changelog wording
  1.15.1
  Wrap register_device coroutine in an ensureDeferred (#7684)
  Ensure the body is a string before comparing push rules. (#7701)
  Ensure etag is a string for GET room_keys/version response (#7691)
  Update m.id.phone to use 'phone' instead of 'number' (#7687)
  Fix "There was no active span when trying to log." error (#7698)
  Enable 3PID add/bind/unbind endpoints on r0 routes
  Discard RDATA from already seen positions. (#7648)
  Replace iteritems/itervalues/iterkeys with native versions. (#7692)
  Fix warnings about losing log context during UI auth. (#7688)
  Fix a typo when comparing the URI & method during UI Auth. (#7689)
  Remove "user_id" from GET /presence. (#7606)
  Increase the default SAML session expirary time to 15 minutes. (#7664)
  fix typo in sample_config.yaml (#7652)
  Take out a lock before modifying _CACHES (#7663)
  Add option to enable encryption by default for new rooms (#7639)
  Clean-up the fallback login code. (#7657)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@richvdh richvdh richvdh approved these changes

Assignees

@richvdh richvdh

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@clokep @richvdh