aws-amplify / amplify-ui

Amplify UI is a collection of accessible, themeable, performant React (and more!) components that can connect directly to the cloud.
https://ui.docs.amplify.aws
Apache License 2.0
884 stars 280 forks source link

Picker from aws-amplify-react cannot pass theme. #1190

Closed SeamusY closed 2 years ago

SeamusY commented 2 years ago

Before opening, please confirm:

JavaScript Framework

React

Amplify APIs

Not applicable

Amplify Categories

Not applicable

Environment information

``` # Put output below this line "@aws-amplify/ui-react": "^2.1.9", "@coreui/coreui": "^2.1.12", "@coreui/coreui-plugin-chartjs-custom-tooltips": "^1.3.1", "@coreui/icons": "0.3.0", "@coreui/react": "^2.5.1", "@stripe/react-stripe-js": "^1.1.2", "@stripe/stripe-js": "^1.7.0", "amazon-connect-chatjs": "^1.1.7", "amazon-connect-streams": "^1.7.5", "aws-amplify": "^4.3.4", "aws-amplify-react": "^2.5.4", "bootstrap": "^4.3.1", "chart.js": "^2.8.0", "classnames": "^2.2.6", "core-js": "^3.1.4", "enzyme": "^3.10.0", "enzyme-adapter-react-16": "^1.14.0", "flag-icon-css": "^3.3.0", "font-awesome": "^4.7.0", "node-sass": "^4.12.0", "prop-types": "^15.7.2", "react": "^16.8.6", "react-app-polyfill": "^1.0.1", "react-art": "^16.11.0", "react-art-svg-renderer": "^0.0.1-rc.8", "react-base-table": "^1.9.1", "react-chartjs-2": "^2.7.6", "react-dom": "^16.8.6", "react-file-viewer": "^1.1.0", "react-medium-image-zoom": "^3.1.2", "react-phone-input-2": "^2.14.0", "react-qr-code": "^0.1.3", "react-router-config": "^5.0.1", "react-router-dom": "^5.0.1", "react-stripe-checkout": "^2.6.3", "react-stripe-elements": "^6.0.1", "react-test-renderer": "^16.8.6", "react-toastify": "^5.4.1", "reactstrap": "^8.0.0", "simple-line-icons": "^2.4.1" ```

Describe the bug

I have attempted to provide a theme to the component, however... the default amazonOrange overrides my theme provided.

Expected behavior

I expect that the theme would enable the background colour of the button to appear as "Blue"

Reproduction steps

  1. yarn
  2. yarn start
  3. Look at the picker.

Code Snippet

// Put your code below this line.

// Also have attempted with import { defaultTheme, AmplifyProvider } from "@aws-amplify/ui-react";
const themeAtp1 = {...AmplifyTheme, 
                  button: {...AmplifyTheme.button, backgroundColor: "green", borderColor: "red"},
                  input: {...AmplifyTheme.input, backgroundColor: "blue", borderColor: "red"}};
  const themeAtp2 = { // Attempted from https://ui.docs.amplify.aws/theming#theme-object
    //   name: 'my-theme',
    //   tokens: {
    //     colors: {
    //       font: {
    //         primary: { value: 'lightblue' },
    //         // ...
    //       },
    //     },
    //   },
    // };
<AmplifyProvider theme={themeAtp2}> // No Changes
                    <p>Text Here</p>
 </AmplifyProvider>

              <Picker
                  headerHint={'Upload Documents as Image'}
                  headerText={'Documents'}
                  preview
                  title={'Select Document'}
                  onPick={file => this.setState({ file })}
                  theme={themeAtp1}
                />

Log output

``` // Put your logs below this line ```

aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

Screen Shot 2022-01-25 at 1 52 03 PM
dbanksdesign commented 2 years ago

At first glance it appears the issues is you could be mixing 2 versions of Amplify UI components @aws-amplify/ui-react and aws-amplify-react packages, which have different theming mechanisms.

SeamusY commented 2 years ago

Originally i stuck with the “aws-amplify-react” which i dug into library, but still looking at the structure… it wasn’t clear how to pass. Upon stack overflow, most of them follow the structure i was given using themeAtp1

Yours Faithfully, S

On 25 Jan 2022, at 17:38, Danny Banks @.***> wrote:

 At first glance it appears the issues is you could be mixing 2 versions of Amplify UI components @aws-amplify/ui-react and aws-amplify-react packages, which have different theming mechanisms.

— Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you authored the thread.

wlee221 commented 2 years ago

Hello @SeamusY, for clarity we're mixing up three versions:

  1. aws-amplify-amplify's PhotoPicker / Picker. This is the one you're using, and it's no longer maintained.
  2. @aws-amplify/ui-react@v1's PhotoPicker. This is the last working version we supported.
  3. @aws-amplify/ui-react@v2 was just released last November. This adds AmplifyProvider, primitives, and Authenticator but not ImagePicker yet.

<Picker /> from (1) will not work with (3) as @dbanksdesign mentioned.

For now, you can either stick to (1) and (2) and theme it yourself. I suggest sticking to (1) because i recall it being more customizable than (2).

For more resilient solution, please consider opening a feature request to this repo for Picker support in latest @aws-amplify/ui-react (3). We would love to bridge this gap and provide first class support for S3 components like we did for Authenticator.

SeamusY commented 2 years ago

Ok, for now. I prefer sticking to (1) any idea on how to customise it?? Ill put in a feature request in (3) later. We may move things to that later to avoid no longer maintained libraries.

Yours Faithfully, Seamus Yeo

On 25 Jan 2022, at 18:28, William Lee @.***> wrote:

 Hello @SeamusY, for clarity we're mixing up three versions:

aws-amplify-amplify's PhotoPicker / Picker. This is the one you're using, and it's no longer maintained. @@.'s PhotoPicker. This is the last working version we supported. @@. was just released last November. This adds AmplifyProvider, primitives, and Authenticator but not ImagePicker yet. from (1) will not work with (3) as @dbanksdesign mentioned.

For now, you can either stick to (1) and (2) and theme it yourself. I suggest sticking to (1) because i recall it being more customizable than (2).

For more resilient solution, please consider opening a feature request to this repo for Picker support in latest @aws-amplify/ui-react (3). We would love to bridge this gap and provide first class support for S3 components like we did for Authenticator.

— Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you were mentioned.

wlee221 commented 2 years ago

I don't think (1) is covered in shadow dom or anything, so you are able to target them directly with CSS along with some className tricks.

For example, I could customize the button here:

https://codesandbox.io/s/legacy-picker-customization-jq7wz?file=/src/App.js

SeamusY commented 2 years ago

I tried applying class name and css on top, it seems the library doesn’t take anything but amplifyTheme. Which i was confused by the structure. Even applying style directly doesn’t apply either.

Yours Faithfully, Seamus Yeo

On 25 Jan 2022, at 19:43, William Lee @.***> wrote:

 I don't think (1) is covered in shadow dom or anything, so you should be able to target them directly with CSS along with some className tricks.

For example, I could customize the button here:

https://codesandbox.io/s/legacy-picker-customization-jq7wz?file=/src/App.js

— Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you were mentioned.

wlee221 commented 2 years ago

Hmm, have you checked out the codesandbox above? I could get the embedded button to change colors/paddings/etc.

wlee221 commented 2 years ago

You shouldn't need to even deal with amplifyTheme for the picker.

wlee221 commented 2 years ago

My css file for that codesandbox is here: https://codesandbox.io/s/legacy-picker-customization-jq7wz?file=/src/styles.css

SeamusY commented 2 years ago

It worked thank you! @wlee221, i suppose the reason this worked is you targeted the parent and then just looked for the first button right?

wlee221 commented 2 years ago

Sounds good. And correct, I tried <Picker className="my-picker" /> first but that didn't work because Picker doesn't pass down the props to its wrapper.

With that, I'll close this issue but please let us know if you find more issues!