canonical / react-components

A set of components based on Vanilla Framework

https://canonical.github.io/react-components

102 stars 55 forks source link

[enhancement] Ability to override logo in LoginPageLayout #1135

Open gruyaume opened 1 month ago

gruyaume commented 1 month ago

Description

This is an enhancement request to add the ability to override the logo in the LoginPageLayout.

If we want to build a login page that does not have the Canonical branding / orange tag, it's not trivial to do so with the existing LoginPageLayout. The Navigation already has this support and here the request is to do the same for the LoginPageLayout.

ashish1233 commented 4 weeks ago

hey @gruyaume , i am new here, i was going through your request.

please correct me if i am wrong

It seems like there is an option to override the logo since, as shown in the image below, we have the ability to pass the logo as a prop. However, while reviewing the Storybook and documentation, I couldn't find any reference or example demonstrating this functionality, so is your request to update the story book?

opheliagame commented 3 weeks ago

@gruyaume @ashish1233 Hi, I started looking at this too and a couple of things bother me currently.

OverridingTheLogo Story in Navigation, does successfully override how the logo looks but we lose the functionality of being able to click on the logo and return to the home page.
The logo prop being used in Navigation and the one being used in LoginPageLayout is not standardized. Maybe this is to avoid dependencies between these two components, but it does make more sense to me for both of these logos to have the same functionality and provide the same interface.

Would love to hear thoughts on this.

bartaz commented 3 weeks ago

The fact that the Navigation supports overriding logos is just for it to be backwards compatible with the times before orange tag logos. It has been deprecated in Vanilla CSS.

The components implemented here were mainly meant for Canonical use (@canonical/react-components), and that's why the Canonical branding (in terms of logo, and all the other styling decisions) is built in by default.

Design System team (Vanilla) is currently planning and starting working on new architecture of the design system, and having a way to use an "unbranded" version is one of the considerations there.

If there is a need for a stronger customisation of the Login page, maybe a solution similar for Navigation could work, but it will definitely be more of a workaround than a systematic solution.

bartaz commented 3 weeks ago

Actually, looking at the LoginPageLayout code it seems that it just reuses Navigation internally, so exactly the same way of overriding the logo should work out of the box:

https://github.com/canonical/react-components/blob/3879dc0cab7cc1b0e6d59792a010b4a0823eb563/src/components/LoginPageLayout/LoginPageLayout.tsx#L48

<LoginPageLayout logo={<img src="…" />} /> should create a LoginPageLayout with a custom logo replacing the orange tag.

ashish1233 commented 3 weeks ago

yeah, This is what i was saying @bartaz , but if you look story book docs there is no logo prop control

so we need to update the Storybook docs

ashish1233 commented 3 weeks ago

like we have in Navigation

bartaz commented 3 weeks ago

yeah, This is what i was saying @bartaz , but if you look story book docs there is no logo prop control

so we need to update the Storybook docs

Yes, it would be worth adding a separate example with logo override, or at least mention in the docs a link to Navigation documentation on this.

bartaz commented 3 weeks ago

like we have in Navigation

What you are showing here @ashish1233 is just an ability to change values of the Canonical logo (the image inside of the orange tag, and text beside it)

What is requested is completely overriding this logo with another image. This can be done as documented here ("Overriding the logo"): https://canonical.github.io/react-components/?path=/docs/components-navigation--docs#overriding%20the%20logo