Update the UI of login page #97
Conversation
There was a problem hiding this comment.
Welcome to the Stave community and good work on the first PR.
As an open-source project, there are a few guidelines that we are trying to follow, which can greatly help the communication and collaboration of the community. Here are a few things that are related to this PR:
- Please fill in the PR template and provide relevant information. For a UI change, uploading an image is very helpful. You can directly paste the image in the editor box.
- When you fill in the template, you will see the issue link. We require each PR to be related to an issue. So if you implement a new feature, you should open an issue. I opened the issue here for you: Extra corner annotation container #79 this time.
- Understanding the build status. You can see a red cross attached to your PR. This indicates the build status. The build need to be passed (green tick), in order for the PR to be accepted. A failed build means there are problems in the code (warnings or errors),
To see the details of the build, click the "Details" link as in the above screenshot. - Currently, you need to resolve the following build warnings:
./src/app/App.tsx
Line 7:3: 'Link' is defined but never used @typescript-eslint/no-unused-vars
Line 93:10: 'Logout' is defined but never used @typescript-eslint/no-unused-vars
./src/app/pages/Login.tsx
Line 7:8: 'Typography' is defined but never used @typescript-eslint/no-unused-vars
Line 56:11: img elements must have an alt prop, either with meaningful text, or an empty string for decorative images jsx-a11y/alt-text
These can be easily solved, most can be solved by removing the references that are not used. And provide alt text to img elements (The purpose of alt text is to allow the screen reader to read the images, which can help people who cannot see or read the screen).
Again, thanks for the first PR to Stave.
|
You also needed to resolve the conflicts here:
|
|
I saw the "remember me" option at the login page. It seem it didn't work at the moment. What's the plan for this? One option is we comment out this for now until we implemented it. Besides that, I have to search for the word "Bienvenue" myself. Any particular reasons for having that word in the signup page? |
|
And we store the logos in the following path, so don't need to upload the logo again to the |
There was a problem hiding this comment.
I think the only thing lefts are:
- Comment out the "Remember me" for now since it does not function.
- Remove the newly uploaded image and replace it with the one here "https://github.com/asyml/stave/tree/master/public"
All fixed. |
src/app/pages/Login.tsx
Outdated
| </div> | ||
| <Container component="main" maxWidth="xs"> | ||
| <div className={classes.paper}> | ||
| <img alt='stave logo' src='https://github.com/asyml/stave/blob/master/public/[email protected]?raw=true'/> |
There was a problem hiding this comment.
The correct way to access the file to use the public URL like this:
https://create-react-app.dev/docs/using-the-public-folder/
The difference is, the project wouldn't rely on the service status of "Github". Instead, the logo file will be deployed to the user's server.
There was a problem hiding this comment.
Or you can follow the following:
https://create-react-app.dev/docs/adding-images-fonts-and-files/
But using the github link is not recommended.
* Add login page * Update Login.tsx * Update Login.tsx * Update App.tsx * Update App.tsx * Fix "ReferenceError: Link is not defined" * Roll back changes on App.tsx * Remove the "Remember me" part and update the logo url * logo update 1. used public url 2. removed the uploaded image Former-commit-id: 03dadb5b13e1fda3b9ed9697d3518833102e3e07
This PR fixes #98.
Description of changes
Updating the UI of the login page by importing Material-UI components:
Possible influences of this PR
No negative influence expected
Demonstration of results
Before:
After: