6 update layout - update colors, change logos #7
Conversation
WalkthroughThe pull request includes various changes across multiple files, primarily focusing on minor formatting adjustments, dependency updates, and visual modifications. Key updates include changes to YAML syntax in configuration files, restructuring of the CHANGELOG for clarity, and updates to branding in UI components. The modifications also involve the addition and removal of specific imports, as well as styling adjustments to enhance the overall presentation of the application. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SignIn
participant SignUp
participant Footer
User->>SignIn: Access Sign-In Page
SignIn->>User: Display Sign-In Interface with LNCO Logo
User->>SignUp: Access Sign-Up Page
SignUp->>User: Display Sign-Up Interface with LNCO Logo
User->>Footer: View Footer
Footer->>User: Show Updated Copyright Notice
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
src/components/SignUp.tsx (1)
Line range hint
1-230: Removal ofuseThemehookI noticed that the
useThemehook has been removed from the imports. This change suggests that the component no longer relies on the theme context for styling or other purposes. Removing unnecessary dependencies can simplify the component and potentially reduce the complexity of the styling logic.Please ensure that the removal of the
useThemehook does not have any unintended consequences or break any existing functionality.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
public/epfl-logo.svgis excluded by!**/*.svgpublic/lnco-logo.pngis excluded by!**/*.pngyarn.lockis excluded by!**/yarn.lock,!**/*.lock
Files selected for processing (9)
- .github/workflows/release-please.yml (1 hunks)
- .yarnrc.yml (1 hunks)
- CHANGELOG.md (1 hunks)
- package.json (1 hunks)
- src/Root.tsx (1 hunks)
- src/components/Footer.tsx (1 hunks)
- src/components/FullscreenContainer.tsx (2 hunks)
- src/components/SignIn.tsx (5 hunks)
- src/components/SignUp.tsx (3 hunks)
Files skipped from review due to trivial changes (3)
- .github/workflows/release-please.yml
- .yarnrc.yml
- src/components/Footer.tsx
Additional context used
LanguageTool
CHANGELOG.md
[grammar] ~363-~363: Did you mean “Google Analytics”?
Context: ...bfbbeef0bceaf79)) - analytics: send google analytics when in prod ([b2f90be](https://github....(GOOGLE_PRODUCTS)
[grammar] ~381-~381: Did you mean “signing”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ...99abfbaddd62e34e14552bb1932f2)) - allow to sign in by typing enter ([3b6ed4a](https://g...(ALLOW_TO)
[misspelling] ~390-~390: This expression is normally spelled as one or with a hyphen.
Context: ...e8)) ### Bug Fixes - add notifier for non existent password ([bcd10da](https://github.com/...(EN_COMPOUNDS_NON_EXISTENT)
Additional comments not posted (28)
src/Root.tsx (3)
22-22: Excellent explanation!The comment provides a clear and concise explanation of the approach to use the Graasp SDK with a custom style while retaining the Language Selector. It helps in understanding the code that follows.
24-24: Removal of Language Selector margin styles.The change to the
languageSelectSxprop, setting it to an empty object, effectively removes the margin styles previously applied to the Language Selector. This aligns with the goal of using the Graasp SDK with a custom style while retaining the Language Selector functionality.
25-25: Verify the impact of disabling the color scheme.Setting the
enableColorSchemeprop tofalsedisables the color scheme behavior of the application. This change may affect the overall styling and appearance of the application.Please ensure that disabling the color scheme does not introduce any inconsistencies or undesired styling changes across the application. Review the application's styling thoroughly to maintain a consistent and cohesive visual experience.
Run the following script to search for potential color scheme usage in the codebase:
src/components/FullscreenContainer.tsx (3)
16-16: LGTM!The change in background color to a semi-transparent black (
rgba(0,0,0,0.08)) aligns with the PR objective of updating the colors in the auth component. This may enhance the visual depth and contrast against the content.
28-28: LGTM!The change in border color to a semi-transparent dark gray (
rgba(40, 40, 40, 0.4)) aligns with the PR objective of updating the colors in the auth component. This may create a softer visual effect.
33-33: LGTM!The change in box shadow to a darker gray (
rgba(40, 40, 40, 0.3)) aligns with the PR objective of updating the colors in the auth component. This may contribute to a more cohesive and modern aesthetic.package.json (3)
18-18: LGTM!The addition of the
@graasp/stylis-plugin-rtldependency with version^2is a good choice to support RTL text rendering in the UI. The version range allows for future compatible updates.
20-20: Verify the reasoning behind using a custom UI library.The change from a specific version of
@graasp/uito a GitHub URL suggests that a custom or updated version of the UI library is being used. While this allows for more control over the UI, it's important to consider the following:
- Ensure that the custom UI library is thoroughly tested and maintained to avoid potential issues.
- Regularly merge updates from the main branch of
@graasp/uito benefit from bug fixes and improvements.- Document the reasoning behind using a custom version and the specific customizations made.
Could you provide more context on why a custom version of the UI library is necessary and how it will be maintained?
27-27: Verify compatibility with the project's React version.The addition of the
lucide-reactdependency is a good choice to provide a consistent set of icons for the UI. The multiple version range allows for flexibility in choosing a compatible version.Please ensure that the chosen version of
lucide-reactis compatible with the React version used in the project to avoid any potential issues.src/components/SignUp.tsx (3)
11-11: LGTM!The
Stackcomponent import from@mui/materialis correctly added and follows the existing import style in the file.
161-163: LGTM!The changes to the logo and layout are implemented correctly:
- The
Stackcomponent is used effectively to vertically stack and center align the logo and header.- The LNCO logo is directly used as an
imgelement, simplifying the code.- The
spacingprop adds consistent vertical spacing between the elements.These changes align with the PR objective of updating the logo and enhancing the layout.
214-217: LGTM!The changes to the sign-in link are implemented correctly:
- The
Linkcomponent is used appropriately to create an internal link to the sign-in page.- The
toprop correctly includes theSIGN_IN_PATHconstant and the current search params.- The inline
styleprop sets the text color of the link to black, ensuring consistency with the design requirements.src/components/SignIn.tsx (5)
5-5: LGTM!The
Buttoncomponent import from@graasp/uilooks good.
8-8: LGTM!The
Stackcomponent import from@mui/materiallooks good.
274-280: LGTM!The changes to display the "LNCO Logo" using an
imgtag and centering the header text using thesxprop look good.
291-291: LGTM!Setting the
colorprop of theButtoncomponent to 'primary' looks good.
300-300: LGTM!Setting the
colorprop of theButtoncomponent to 'primary' looks good.CHANGELOG.md (11)
9-22: LGTM!The bug fixes for version 2.5.2 include:
- Translation updates for Arabic, French, German, and Spanish languages, which enhance the localization support.
- Dependency updates for @graasp/query-client, i18next, react-i18next, and mui packages, ensuring the project stays up to date.
- Update to the release-please workflow, suggesting improvements to the release management process.
28-42: LGTM!The bug fixes for version 2.5.1 include:
- Dependency updates for various @graasp packages, @sentry/react, react-i18next, and mui, ensuring the project stays up to date.
- Addition of Italian and Spanish translations, enhancing the localization support.
- Fix to send language on register, improving the user experience by using the correct language for registered users.
49-54: LGTM!Version 2.5.0 introduces a new feature and includes bug fixes:
- New feature to regroup the API Alert with the Form container, improving the user interface by grouping related components.
- Dependency updates for @graasp/ui and i18next packages, ensuring the project stays up to date.
60-67: LGTM!Version 2.4.0 introduces a new feature and includes bug fixes:
- New feature to add an enable analytics checkbox for signup, enhancing user privacy by allowing them to opt-in or opt-out of analytics during registration.
- Dependency updates for @emotion/styled, @graasp/translations, @sentry/react, and mui packages, ensuring the project stays up to date.
73-85: LGTM!Version 2.3.1 includes a bug fix that updates loading buttons and adds error messages, improving the user experience by providing better feedback during user interactions.
79-85: LGTM!Version 2.3.0 introduces new features and includes bug fixes:
- New feature to ask users to read and accept terms of usage and privacy policy, ensuring compliance with legal requirements and improving user awareness.
- New feature to fix styles, show logo, and add footer content, enhancing the visual appearance and branding of the application.
- Bug fix to allow no recaptcha key in dev environment, simplifying the development setup.
- Bug fix to use stacked notifications, improving the user experience by presenting notifications in a more organized manner.
91-103: LGTM!Version 2.2.0 introduces a new feature and includes bug fixes:
- New feature to add a connection indicator, improving the user experience by providing visual feedback about the network connection status.
- Dependency updates for various @graasp packages, @sentry/react, react-i18next, react-toastify, and mui, ensuring the project stays up to date.
109-119: LGTM!The bug fixes for version 2.1.2 include:
- Dependency updates for @emotion/react, @graasp/sdk, @graasp/ui, @sentry/react, i18next, react-i18next, and mui packages, ensuring the project stays up to date.
- Addition of Italian and Spanish translations, enhancing the localization support.
- Fix to send language on register, improving the user experience by using the correct language for registered users.
125-128: LGTM!The bug fixes for version 2.1.1 include:
- Dependency updates for @emotion/react, @graasp/query-client, and @graasp/translations packages, ensuring the project stays up to date.
- Update ESM dependencies, improving the compatibility and performance of the project.
134-147: LGTM!Version 2.1.0 introduces a new feature and includes bug fixes:
- New feature to import translations, enhancing the localization support of the project.
- Dependency updates for @graasp/query-client, @graasp/translations, @graasp/ui, @sentry/react, and react-router packages, ensuring the project stays up to date.
- Update the release-please condition, improving the release management process.
- Use the test environment in Cypress open, ensuring consistent testing across different environments.
153-170: LGTM!The bug fixes for versions 2.0.2 and 2.0.1 include dependency updates for various packages such as @graasp/query-client, @graasp/sdk, @graasp/translations, @graasp/ui, @mui/lab, react-i18next, and sentry, ensuring the project stays up to date.
src/components/SignIn.tsx
Outdated
| <Link style={{ color: '#000' }} to={`${SIGN_UP_PATH}${search}`}> | ||
| {t(SIGN_UP_LINK_TEXT)} | ||
| </Link> |
There was a problem hiding this comment.
Consider using CSS classes or a styling library for consistency.
Setting the color directly in the style prop is not the recommended approach in React. It's better to use CSS classes or a styling library like styled-components for consistency and maintainability.
For example, you can create a styled Link component using styled-components:
import styled from 'styled-components';
import { Link } from 'react-router-dom';
const StyledLink = styled(Link)`
color: #000;
`;Then, use the StyledLink component instead:
<StyledLink to={`${SIGN_UP_PATH}${search}`}>
{t(SIGN_UP_LINK_TEXT)}
</StyledLink>
| @@ -2,11 +2,11 @@ | |||
| <html lang="en"> | |||
| <head> | |||
| <meta charset="UTF-8" /> | |||
| <link rel="icon" type="image/svg+xml" href="/graasp.svg" /> | |||
| <link rel="icon" type="image/png" href="/lnco-logo.png" /> | |||
There was a problem hiding this comment.
When do you think we can have an SVG version?
There was a problem hiding this comment.
So it's definitely not very simple, I will try one more approach myself today otherwise we will have to ask around. I would suggest for now that this is not a blocker for this PR.
| sx={{ fill: theme.palette.primary.main }} | ||
| /> | ||
| <Typography variant="h4" component="h2" id={SIGN_IN_HEADER_ID}> | ||
| <img alt="LNCO Logo" height="200" src="/lnco-logo.png" /> |
There was a problem hiding this comment.
I would try to include this as an SVG component.
| <Stack spacing={1}> | ||
| <GraaspLogo height={90} sx={{ fill: theme.palette.primary.main }} /> | ||
| <img alt="LNCO Logo" height="200" src="/lnco-logo.png" /> |
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
src/config/constants.ts (1)
2-2: LGTM! Consider adding a comment for clarity.The new constant
AUTH_BOX_BORDER_AND_SHADOW_COLORis well-named and uses an appropriate RGBA color value. It will be useful for maintaining consistent styling for the authentication box.Consider adding a brief comment above the constant to explain its purpose or where it should be used. This can help other developers understand its intended use at a glance. For example:
// Color for the border and shadow of the authentication box export const AUTH_BOX_BORDER_AND_SHADOW_COLOR = 'rgba(40,40,40,0.4)';
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- package.json (1 hunks)
- src/components/FullscreenContainer.tsx (3 hunks)
- src/config/constants.ts (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- package.json
- src/components/FullscreenContainer.tsx
|
@juancarlosfarah I updated your comments except for creating the SVG. This is work in progress, but it's not that easy and quite time-consuming, so I don't think it should be blocking for this PR. |
Updated layout of the auth component including:
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Documentation