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

Finished redesigning Issue 5 Exhibit #115

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

JacobYLiu
Copy link

Added Typography and border-radius of two images.
Changed Learn-style.css and Learn-style.jsx

Added Typography and border radius of two images.
@netlify
Copy link

netlify bot commented May 15, 2023

Deploy Preview for orcahome ready!

Name Link
🔨 Latest commit 47f5bd4
🔍 Latest deploy log https://app.netlify.com/sites/orcahome/deploys/64701b3b8d39c50008154260
😎 Deploy Preview https://deploy-preview-115--orcahome.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

package.json Outdated
@@ -22,17 +22,17 @@
"@emotion/styled": "^11.6.0",
"@mui/icons-material": "^5.3.1",
"@mui/material": "^5.3.1",
"next": "^12.0.9",
"react": "17.0.2",
"next": "^13.3.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

would be beneficial to stay with original versions of Next, React, etc.. would recommend omitting this file unless there's a feature that is not currently implemented in the previous versions

Copy link
Author

Choose a reason for hiding this comment

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

Oh okay, I will try omitting the file. I think the versions got updated after running yarn or npm.

JacobYLiu added 14 commits May 25, 2023 10:41
Trying to see if it would work.
Omitted package.json
I updated the versions manually in my local package.json based on orcahome main branch's package.json file.

Ran npm install and npm ci to double check npm error messages on my local computer.

Came to error messages of npm audit severity on vulnerabilities. I'm committing my changes before I try npm audit fix.
I didn't need to run npm audit fix. Pull request checks/jobs fixed after running npm prettier.

https://stackoverflow.com/questions/69797806/code-style-issues-found-in-the-above-files-forgot-to-run-prettier
Removed lock.json to fix netlify build
Had to change Omit 'onTouchStart' as well. Then I ran npm run build to check if build runs through.
@JacobYLiu
Copy link
Author

Okay, finally. All the rules are passed and CI checks are good. I think it is ready to review and merge with the main repo.

@JacobYLiu JacobYLiu closed this May 26, 2023
@JacobYLiu JacobYLiu reopened this May 26, 2023
@UXBrendan
Copy link

@JacobYLiu

Please see images below. The Exhibit section needs to be fixed- the exhibit cards need to be closer to the title "Exhibit". Please set the spacing closer- let me know if you have any questions. Also, please change name to "Exhibits".

Screenshot 2023-09-14 at 4 27 24 PM
Screenshot 2023-09-14 at 4 27 36 PM

@UXBrendan
Copy link

UXBrendan commented Sep 18, 2023

@JacobYLiu

See image- space between "Exhibits" title and card should be 80 pixels.

Figma file: https://www.figma.com/file/YdvbAjHETokV2x1LBHaElj/Orcahome-%2F-Orcasound-Website-Redesign-2021?type=design&node-id=298-290&mode=dev

Screenshot 2023-09-18 at 1 39 17 PM

!.vscode/*.shared*package.json

Choose a reason for hiding this comment

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

This doesn't seem correct. I would not expect there to be a .sharedpackage.json file as well it removes the .shared folder

what were you intending to do with this edit?

Choose a reason for hiding this comment

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

This seems best to not add to the repo


// Add support for the sx prop for consistency with the other branches.

Choose a reason for hiding this comment

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

remove, seems unnecessary spacing.

text-align: left;
justify-content: left;
width: 100%;
/* flex-wrap: wrap; */

Choose a reason for hiding this comment

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

remove commented styles

border-radius: 10%;
}
.org :nth-of-type(2) {
/* border: solid 1px; */

Choose a reason for hiding this comment

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

remove commented style.

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

Successfully merging this pull request may close these issues.

4 participants