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

285 new css styling option on cards #293

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

Conversation

jonatan-lledo-netcentric
Copy link
Collaborator

@jonatan-lledo-netcentric jonatan-lledo-netcentric commented Feb 21, 2023

for the 1st and 2nd style issues, it can be check in index, and for the 3rd, Y add a draft for the purposes in
/drafts/jlledo/index-demo
/drafts/jlledo/rainbow-demo

Fix #285

Test URLs:

@jonatan-lledo-netcentric jonatan-lledo-netcentric linked an issue Feb 21, 2023 that may be closed by this pull request
3 tasks
@aem-code-sync
Copy link

aem-code-sync bot commented Feb 21, 2023

Hello, I'm Franklin Bot and I will run some test suites that validate the page speed.
In case there are problems, just click the checkbox below to rerun the respective action.

  • Re-run PSI Checks

@aem-code-sync
Copy link

aem-code-sync bot commented Feb 21, 2023

Page Score PSI Audit Google
/ SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link

aem-code-sync bot commented Feb 21, 2023

Page Score PSI Audit Google
/ SI FCP LCP TBT CLS PSI

@pedrogbot
Copy link
Collaborator

pedrogbot commented Feb 21, 2023

The following functionalities have been tested on this ticket:

  • Enable colours settings on components background (ex: Social media cards) -> Working as expected
  • Enable colours settings on (Cards) components for titles, subtitles and body text to white when background colours are medium or dark -> Working as expected
  • Darkening Filter -> When the background used in this component is a photo, the 110% brightness makes the texts unreadable and the images do not look good. In addition, this component is often used on the website with different colours, so the change in brightness affects them differently depending on the colour used. When hovering over the component, it looks even worse.

Please find attached some of the pages where the affected component is used and you can see the behaviour, on the left side the page in production and on the right side how it looks with the new changes in the branch.

image

image

image

image

image

image

When the behaviour of the brightness filter component is decided, all functionalities will be reviewed again.

@pedrogbot
Copy link
Collaborator

As we have discussed on the slack channel, by applying the same component on photos and colours it will work on colours but on photos it will look too bright and the text will not be legible, we should apply one solution for colours (which is the new brightness that is being developed) and create a new flavour for images, so they look correctly

@NCalexiaeche
Copy link
Collaborator

This ticket excludes the images, this block should be used only for background plain color, it should not be possible to be used with any images

@alexnerdrumhansen
Copy link
Collaborator

Colours don't seem to be matching the accessibility approved colours, https://projects.netcentric.biz/wiki/display/NCWEB/Accessibility+colour+recommendation
Should be: Midnight Blue (#48) / Dark Plum (#2E308E) / Dark Blue (#2F78C4)

@aem-code-sync
Copy link

aem-code-sync bot commented Oct 6, 2023

Hello, I'm the AEM Code Sync Bot and I will run some test suites that validate the page speed.
In case there are problems, just click the checkbox below to rerun the respective action.

  • Re-run PSI Checks

@aem-code-sync
Copy link

aem-code-sync bot commented Oct 6, 2023

Page Scores Audits Google
/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@jonatan-lledo-netcentric
Copy link
Collaborator Author

@tomasznetcentric please, if you think this PR is ready, merge it for me. thanks

@tomasznetcentric
Copy link
Collaborator

@tomasznetcentric please, if you think this PR is ready, merge it for me. thanks

I checked the pr code wise. But yes maybe I miss some context here. Will setup a alignment for us.

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.

New CSS styling option on Images & Banner Style color change
6 participants