-
Notifications
You must be signed in to change notification settings - Fork 188
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
Fixes issue-1371: Make the picture sticky in About page #1372
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @lixiaoqity,
Our About page needs some improvements for sure.
Be aware of what is going on 1232 and 1335
I think it's a better idea to contact @Wei-J-Huang and see how you can manage your ideas together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have made the pictures a little below the navbar so that it can not be covered. |
sticky: { | ||
position: 'sticky', | ||
top: '80px', | ||
}, | ||
carousel: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1)Find a good position for the carousel, vertically centred. Then after this, we don't want this carousel moving.
2)Let's scroll only the text part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now the position is moved to the center and fixed. Only text can be scrolled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need just to remove one comment line that you left. And I think we will be good to go.
@@ -37,7 +37,7 @@ const tutorialSteps = [ | |||
|
|||
const useStyles = makeStyles((theme) => ({ | |||
root: { | |||
flexGrow: 1, | |||
// flexGrow: 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this comment. Then I think we are good to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, forget to remove it. I have fixed it. Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@humphd sorry, I forgot to test all the responsive sizes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to make the page responsive.
In case you need the documentation: material-ui-doc
I put the pictures a little close to the above. If it is set on the vertical centre, it will cover the footer. |
The two columns look uncomfortable when the screen size is small. Now I think it is better. |
I pulled a new request at #1486. Please check it. Thanks. |
@PedroFonsecaDEV |
@lixiaoqity Sorry, I must have clicked on a different build. Please rebase (there is a conflict) so we can check the build after a rebase. |
This pull request should be discarded. I have created a new pull request #1486 which is rebased. Thanks. |
Issue This PR Addresses
Fix #1371
Type of Change
Description
In About page, when we scroll down, half of the page will be blank now. It looks weird. So, I want to make the pictures sticky.
Checklist