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

COMDOX-840: Redesign Feedback component #269

Merged
merged 6 commits into from
Jan 16, 2024

Conversation

jeff-matthews
Copy link
Contributor

@jeff-matthews jeff-matthews commented Dec 21, 2023

Purpose of this pull request

This pull request (PR) applies custom styling to the Feedback component. It requires shadowing the MDXFilter component in the gatsby-aio-theme so that the styling can be applied to the parent <div> that the Feedback component lives in.

Note

Applying the styling to the Feedback component did not work for me.

The only changes I made to the MDXFilter component include:

  • Update import directives from relative links (e.g., ../<component-name>) to local node module links (e.g., @adobe/gatsby-theme-aio/ssrc/components/<component-name>)
  • Add custom CSS to parent <div> that contains the Feedback component (lines 373-380)

I also had to copy the conf/globals.js file from the theme to this project because the MDXFilter component relies on it and that file does not exist in the node modules.

See COMDOX-840 for more details.

Staging

The styling still needs some work, but at least we now have a working experiment. I'm considering adding a border to differentiate the component from the background text similar to ExL.

I'm also not sure about the Contributors info that currently sits in the same <div>. We may want to separate it from the Feedback component. I'm open to suggestions

https://adobedocs.github.io/commerce-webapi/

@jeff-matthews jeff-matthews self-assigned this Dec 21, 2023
@jeff-matthews jeff-matthews added enhancement New feature or request internal Differentiates work between community and Adobe staff labels Dec 21, 2023
@jeff-matthews
Copy link
Contributor Author

jeff-matthews commented Dec 21, 2023

I added some more styling to make the component more visible (9591704).

New staging build is blocked on an unrelated validation error.

Screenshot 2023-12-21 at 10 15 19 AM

@bdenham had a good suggestion to shadow the Feedback component and add a line of JS to get a reference to it’s parent element and add a CSS class there. That hadn't occurred to me in my initial investigation. I think that would be preferable to shadowing MDXFilter because MDXFilter is a much larger and complex component.

@bdenham, please feel free to create a new branch to demo that concept 😄

@bdenham
Copy link
Contributor

bdenham commented Dec 21, 2023

On it! But first, I've gotta put together my forgotten Q4 reflection! Yikes. Will continue after.

@jeff-matthews
Copy link
Contributor Author

New staging build is available

@jeff-matthews
Copy link
Contributor Author

Design variant proposed in #270 courtesy of @bdenham 😄

Copy link
Contributor

@jhadobe jhadobe left a comment

Choose a reason for hiding this comment

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

looks good! maybe in the future we can iterate the design? (i would prefer less drop shadow, personally.)

@jeff-matthews
Copy link
Contributor Author

jeff-matthews commented Jan 4, 2024

We can definitely iterate @jhadobe. In fact, I'm not opposed to completely dropping the box-shadow property for the initial test.

@hguthrie
Copy link
Contributor

hguthrie commented Jan 4, 2024

Design variant proposed in #270 courtesy of @bdenham 😄

I like the emoji approach with the pale background. It's relatable for a wide range of folks.

As for drop shadows, use only if the contrast is required for some visual benefit. It looks okay the way you have it because it draws more attention than the version on Experience League. However, I think the emoji version would eliminate the need for it.

@jhadobe
Copy link
Contributor

jhadobe commented Jan 4, 2024

Design variant proposed in #270 courtesy of @bdenham 😄

I like the emoji approach with the pale background. It's relatable for a wide range of folks.

As for drop shadows, use only if the contrast is required for some visual benefit. It looks okay the way you have it because it draws more attention than the version on Experience League. However, I think the emoji version would eliminate the need for it.

i agree that images will definitely draw more attention, but i think single color emotes or thumbs up/down icons might be more stylistically appropriate.

@jeff-matthews
Copy link
Contributor Author

jeff-matthews commented Jan 4, 2024

The emojis are cool, but I don't think we have a mapping for the neutral option in our analytics data layer; only yes or no. So if we go with emojis then I think we'd only be able to have two.

Maybe emojis would be a good follow up iteration if we're not getting the results we expect. I'm not sure how bold our first iteration should be. My instinct is to start with more subtlety. I like @jhadobe's suggestion re: thumbs up/down instead, but it will require shadowing an additional component.

@jeff-matthews jeff-matthews marked this pull request as ready for review January 11, 2024 21:27
@jeff-matthews
Copy link
Contributor Author

Per team discussion on 1/11/24, consensus is to start with conservative design improvements (lower tech debt) and make smaller iterations (limit potential UX shock and negative reaction).

@jeff-matthews
Copy link
Contributor Author

@timkim, shadowing in this repo is only meant to be a temporary situation while we run our experiment (1-2 months). Depending on the outcome, we may make a formal request to add some config-based options to the theme.

Please let me know if you see anything that you think should be addressed before I merge.

@jeff-matthews jeff-matthews merged commit 747f4e6 into main Jan 16, 2024
15 checks passed
@jeff-matthews jeff-matthews deleted the COMDOX-840-feedback-component branch January 16, 2024 15:24
@timkim
Copy link
Contributor

timkim commented Jan 17, 2024

@timkim, shadowing in this repo is only meant to be a temporary situation while we run our experiment (1-2 months). Depending on the outcome, we may make a formal request to add some config-based options to the theme.

Please let me know if you see anything that you think should be addressed before I merge.

Oh oop - I forgot to come back to this. I think what we can do is add a css class name to the feedback component or parent div that it lives in. And have a css file in the repo that references those class names so you can override any styles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request internal Differentiates work between community and Adobe staff
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants