Skip to content

Resource dropdown boxes & rendering fake data #50

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
Merged
merged 12 commits into from
Aug 17, 2022
Merged

Resource dropdown boxes & rendering fake data #50

merged 12 commits into from
Aug 17, 2022

Conversation

@KHVBui
Copy link
Collaborator

@KHVBui KHVBui commented Aug 11, 2022

  • Installed react-youtube for future video dropdowns in tutorials
  • Installed prop-types to resolve linter props validation error
  • render fakedata for resource tabs with appropriate CSS for list items

@KHVBui
Copy link
Collaborator Author

KHVBui commented Aug 11, 2022

Current appearance. Let me know if anything needs to be changed.
Screen Shot 2022-08-10 at 8 31 12 PM

@KHVBui KHVBui self-assigned this Aug 11, 2022
@KHVBui KHVBui requested a review from minhngo3818 August 11, 2022 07:03
@KHVBui KHVBui added CSS require using css Resources-page resouces page React Require to use React js Frontend task relates to frontend feature-request request a new feature not ready PR PR is not ready to be merged labels Aug 11, 2022
minhngo3818
minhngo3818 reviewed Aug 13, 2022
View reviewed changes
Copy link
Collaborator

@minhngo3818 minhngo3818 left a comment

Choose a reason for hiding this comment

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

There is an mistake with padding & margin I made in the tab content, so I will fix it later. I think the styling for title should be changed

KHVBui added 5 commits August 15, 2022 14:34
@KHVBui
Install prop-types and react-youtube
90151dd 
- react-youtube for later video dropdowns
- prop-types resolves linter errors for typing
@KHVBui
Refactor Resources using Minh Ngo's code
a16bee4 
- Code from Pull Request #31, with edits since rendering occurs in separate files
- Reduces repetitive code for creating individual buttons
@KHVBui
Render fakeData for Tutorials
4a9412d
@KHVBui
Render fakeData for Readings
0ef6d02
@KHVBui
Render fakeData for Organizations
7c01ba4
@KHVBui KHVBui force-pushed the Resource-Dropdown-Boxes-&-Fake-Data branch from 1a4352d to c7e3b02 Compare August 16, 2022 04:29
@KHVBui
Formatting & Transitions for Resource lists
a3e337f 
- box-shadows changed to border-bottom for clarity
- Transitions added for titles and descriptions
- Padding adjusted for resources-content to be wider
- fixed bug where resources-container wasn't adjusting its height as contents grew
@KHVBui KHVBui force-pushed the Resource-Dropdown-Boxes-&-Fake-Data branch from c7e3b02 to a3e337f Compare August 16, 2022 04:35
@KHVBui
Copy link
Collaborator Author

KHVBui commented Aug 16, 2022

Latest state of the design.
Screen Shot 2022-08-15 at 9 39 21 PM

@KHVBui KHVBui requested a review from minhngo3818 August 16, 2022 04:40
@minhngo3818
Copy link
Collaborator

It's good so far. I think we should discuss general patterns for each page to unify design. I'll start reviewing it

@KHVBui KHVBui removed the not ready PR PR is not ready to be merged label Aug 16, 2022
minhngo3818
minhngo3818 reviewed Aug 16, 2022
View reviewed changes
minhngo3818
minhngo3818 reviewed Aug 16, 2022
View reviewed changes
minhngo3818
minhngo3818 reviewed Aug 16, 2022
View reviewed changes
minhngo3818
minhngo3818 reviewed Aug 16, 2022
View reviewed changes
minhngo3818
minhngo3818 reviewed Aug 16, 2022
View reviewed changes
minhngo3818
minhngo3818 reviewed Aug 16, 2022
View reviewed changes
minhngo3818
minhngo3818 reviewed Aug 16, 2022
View reviewed changes
minhngo3818
minhngo3818 reviewed Aug 16, 2022
View reviewed changes
Copy link
Collaborator

@minhngo3818 minhngo3818 left a comment

Choose a reason for hiding this comment

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

Hey Kevin, there are some redundant codes in className for all three sub pages js file. I gives some comments for css enhancement. Please read through.

KHVBui added 5 commits August 16, 2022 18:39
@KHVBui
Fixed console errors
bbdf063 
- li's can't be under another li, so switched resource-list-description to divs
- Readings need a key for its ToggleItem
@KHVBui
Adjusting line break formatting & active class names
af163d6 
- "resource-list-title-active" and "resource-list-description-active" reduced to just an .active class now
- Instead of using the CSS "white-space: pre-line;" and new line chars (\n), switched to <p> tags
@KHVBui
Adjusting responsiveness & reduce redundancy
166b66f 
resources-container:
- Top margins now don't leave a huge gap on mobile
- max-width to prevent too much stretching
- min-height unnecessary with bottom padding

resources-tabs button
- Used "min" to make title font  responsive since "Organizations" kept getting cut off on smaller screens

resource-content
- % for padding more responsive
- overall box doesn't need a border

resource-list-item & button
- Creates the dividing lines
- override default button formatting (particularly the bezeling)

resource-list-title
- 2rem is too large for padding
- border controlled by resource-list-item now
- font is now responsive

resource-list-title.active
- border controlled by resource-list-item now

resource-list-description
- Newlines are created using <p> now
- font is now responsive

resource-list-description.active
- 2 rem is too much for padding
- border controlled by resource-list-item now

resource-list-description p
- controls formatting of <p>
@KHVBui
Made "Organizations" plural in resource-tabs
592a087
@KHVBui
Remove excess padding descriptions
6561335 
- where the top and bottom are the same and left and right are the same, can reduce to two values
@KHVBui
Copy link
Collaborator Author

KHVBui commented Aug 17, 2022

I think I got all the comments you made Minh. Let me know if there's anything else.

@minhngo3818 minhngo3818 self-requested a review August 17, 2022 17:25
minhngo3818
minhngo3818 reviewed Aug 17, 2022
View reviewed changes
minhngo3818
minhngo3818 requested changes Aug 17, 2022
View reviewed changes
Copy link
Collaborator

@minhngo3818 minhngo3818 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The code and page look pretty nice! There is only one thing that, you need to refactor import of fakeData in three subpages and change mapping in those pages. Edit it and you're ready to go :)

@KHVBui
Cleaning up imports
9b57a1a 
- removed "require" and used "import" instead for clarity
- removed unnecessary "React" import in "Resources"
@KHVBui KHVBui requested a review from minhngo3818 August 17, 2022 20:02
minhngo3818
minhngo3818 approved these changes Aug 17, 2022
View reviewed changes
Copy link
Collaborator

@minhngo3818 minhngo3818 left a comment

Choose a reason for hiding this comment

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

Very nice!

@KHVBui KHVBui merged commit a60f328 into main Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@minhngo3818 minhngo3818 minhngo3818 approved these changes

Assignees

@KHVBui KHVBui

Labels

CSS require using css feature-request request a new feature Frontend task relates to frontend React Require to use React js Resources-page resouces page

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@KHVBui @minhngo3818