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

Homescreen and Infoscreen #11

Merged
merged 45 commits into from Mar 31, 2021
Merged

Homescreen and Infoscreen #11

merged 45 commits into from Mar 31, 2021

Conversation

LinkKF
Copy link
Collaborator

@LinkKF LinkKF commented Mar 27, 2021

This PR adds the Homescreen and the Infoscreen for the website. The mdx "code" to test it lies in the content file "contentPage". The filter from the Homescreen is removed, but I work on getting it implemented again. On the info pages, the additional Infos are, other than we said earlier, aside the picture instead of under it. I think like that its more clear, where it belongs to.

@LinkKF
added rectImage (WIP)
a2fc888
@LinkKF
refactored image components to one component
2fa8b00
@LinkKF
67b7944
@LinkKF
refactored image components to one component
e498bfd
@LinkKF
added image size
8b94b83
@LinkKF
adding picture on left/right side
de2f117
@LinkKF
renamed "shape" to "props"
9edd6e0
@LinkKF
added component homescreen
c31d4c1
@LinkKF
inserting adapting text (no real design yet)
233d44c
@LinkKF
Removed Hello World Text (for testing purposes)
1ec31da
@LinkKF
removed content (for testing purposes)
08d222b
@LinkKF
renamed container
bc1f9bf
@LinkKF
added float left/right
16c35de
@LinkKF
added info component (not yet finished)
47ac3fa
@LinkKF
added textField
f6d789b
@LinkKF
fixed homescreen
d5022ac
@LinkKF
changed from float to flexboxes
cde5404
@LinkKF
Homescreen adaptations
aaddfb6
@LinkKF
added second content page with "Code" for content
b12fbd2
@LinkKF
made text variable
60729a2
@LinkKF
added border radius
ef119ff
@LinkKF
added responsive design
13705c5
@LinkKF
filter:none
7f55237
@LinkKF
added white background to welcome text
5c92cf4
@LinkKF
added additiona Information div for things
add9c8d
@LinkKF
removed additional Info div, when empty
232fcb0
@LinkKF
Merge branch 'development' into new-components
d0b4047
@LinkKF
add contentPage for development purpose
006343f
@LinkKF
""
8bf3641
@LinkKF
added module stuff
da38877
@LinkKF LinkKF requested a review from Quaffel March 27, 2021 14:19
Quaffel
Quaffel requested changes Mar 28, 2021
View reviewed changes
Copy link
Owner

@Quaffel Quaffel left a comment

Choose a reason for hiding this comment

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

Good overall, but I have some requests. Please address them and re-request my review once you're done. And don't forget to resolve the merge conflicts. If you have any questions, please don't mind asking!

frontend/src/components/mdx/textField/textField.tsx Outdated Show resolved Hide resolved
frontend/src/components/mdx/info/info.tsx Outdated Show resolved Hide resolved
frontend/src/components/mdx/info/info.tsx Show resolved Hide resolved
frontend/content/contentPage.mdx Outdated Show resolved Hide resolved
frontend/src/components/mdx/image/image.tsx Show resolved Hide resolved
frontend/src/components/mdx/image/image.tsx Show resolved Hide resolved
frontend/src/components/mdx/homescreen/homescreen.module.scss Outdated
left: 0;
bottom: 0;
right:0;
width:100vw;
Copy link
Owner

Choose a reason for hiding this comment

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

Remove these properties as they'd hide the navbar and anything else preceding it. Keep in mind that you're using position: absolute here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I didn't find away to use the "remaining" height of the screensize. Therefore I had to stick to absolute positioning and bottom: 0. To make the navbar visible the homescreen component is now offset to the background. It will take 100% of the height and width all the time but no more. As I think the main screen of this application will remain quite clean I think it should be alright that way.

Copy link
Owner

Choose a reason for hiding this comment

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

You could have used the flexbox algorithm on the element enclosing the navbar and the homescreen component, but moving the homescreen component to the background should be just fine for now. We might address this in a future PR, however...

frontend/src/components/mdx/homescreen/homescreen.module.scss Outdated Show resolved Hide resolved
frontend/src/components/mdx/homescreen/homescreen.module.scss Outdated Show resolved Hide resolved
frontend/src/components/mdx/info/info.module.scss Outdated Show resolved Hide resolved
@LinkKF LinkKF requested a review from Quaffel March 29, 2021 12:48
Quaffel
Quaffel requested changes Mar 30, 2021
View reviewed changes
Copy link
Owner

@Quaffel Quaffel left a comment •
edited

Choose a reason for hiding this comment

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

Overall good improvement, but please address the issues that are still open.
Would also be great if you could use a proper formatter in the SCSS files (mind the missing space character between the colons and the properties' actual values).

frontend/src/components/mdx/image/image.tsx Show resolved Hide resolved
frontend/src/components/mdx/homescreen/homescreen.module.scss Outdated Show resolved Hide resolved
frontend/src/components/mdx/homescreen/homescreen.module.scss Outdated
left: 0;
bottom: 0;
right:0;
width:100vw;
Copy link
Owner

Choose a reason for hiding this comment

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

You could have used the flexbox algorithm on the element enclosing the navbar and the homescreen component, but moving the homescreen component to the background should be just fine for now. We might address this in a future PR, however...

@Quaffel Quaffel mentioned this pull request Mar 30, 2021
Open
@LinkKF LinkKF requested a review from Quaffel March 30, 2021 17:03
@LinkKF LinkKF requested a review from Quaffel March 30, 2021 18:54
Quaffel
Quaffel requested changes Mar 30, 2021
View reviewed changes
Copy link
Owner

@Quaffel Quaffel left a comment

Choose a reason for hiding this comment

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

I'm really sorry for bothering you again, but I still want you to do two minor things:

  • Please restore the original package.json (The items are rearranged on npm installs)
  • Remove the redundant package-lock.json in the project's root.

I promise, you'll be good to go then 👼

@LinkKF LinkKF requested a review from Quaffel March 30, 2021 20:02
@LinkKF
Copy link
Collaborator Author

LinkKF commented Mar 30, 2021

Finally, WOOHOO

@LinkKF LinkKF merged commit 083a662 into development Mar 31, 2021
@LinkKF LinkKF deleted the new-components branch March 31, 2021 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Quaffel Quaffel Quaffel approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@LinkKF @Quaffel