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.

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

Milestone #1 #1

Closed
Suzii opened this issue Oct 26, 2018 · 5 comments
Closed

Milestone #1 #1

Suzii opened this issue Oct 26, 2018 · 5 comments

Comments

@Suzii
Copy link
Contributor

Suzii commented Oct 26, 2018

Status: ✔️

Below are just suggestions to consider for the next milestones.
Thumbs up for TypeScript 😉
 

Design suggestions

As far as the design goes, the app is a bit broken - you cannot see some characters like 'C' (Channels) etc. The fix is easy - wrap the App component's content with container css class. Generally, make sure you use the bootstrap classes correctly. See this section of documentation to get an overview on how to structure the page with a navigation.
Single message could use more styling - try giving it more vertical space.
 

Code suggestions

I hope to see no code commented out in the final submission 😉
Function arguments should always be camelCase + string type in typescript is 'string' not 'String'. (5603cac#diff-c8a101915dc6871f1f3d7ad2208d8655R25)
 
Image used in profile picture, should not be inlined like this. Use a normal image and import it to react component, then render it in src attribute (https://stackoverflow.com/questions/39999367/how-do-i-reference-a-local-image-in-react).
I see you have some todos to create more components like single message - yes, please do 😄
Const.ts. file should have been rather something like constants/appNames.ts.

@stefanbartos
Copy link
Owner

stefanbartos commented Nov 2, 2018

I tried to use require for static image, but ts-lint displayed error message as follows: require() style import is forbidden so i used base64 encoding (for info about rule). What solution do you propose? @Suzii

@Suzii
Copy link
Contributor Author

Suzii commented Nov 6, 2018

Well, did you try to save the image as jpg/png and then import the file?

@stefanbartos
Copy link
Owner

Yes, i've tried.

@Suzii
Copy link
Contributor Author

Suzii commented Nov 8, 2018

So after a bit of googling, this solution seems to be working without any warnings #2 😉

@mcvincekova
Copy link
Collaborator

Neat.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants