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

feat(banner): banner without title and no clickable #1069

Merged
merged 2 commits into from
Feb 27, 2023

Conversation

christianpe98
Copy link
Contributor

@christianpe98 christianpe98 commented Feb 9, 2023

Motivation

From motive, our intention is to have banners that are not clickable and do not have a title. Right now, the X banner forces any banner to have a URL to redirect to and a title. In this PR we propose a solution to support X banners and the banners Motive needs.

Main changes

banner.model.ts

  • url and title are optional.

banner.vue

  • If the banner has a URL it will be clickable, triggering the appropriate events, otherwise it will just be an image(with or without title).
  • If there is an error loading the banner image, the banner will be removed from the DOM.
  • If the banner does not have a title, the alternative text of the image will be "Banner" and the heading(h2) will not be rendered.

banner.spec.ts

We have defined some unit tests at component level using Jest(x-components/src/x-modules/search/components/__tests__/banner.spec.ts) and others using Cypress(x-components/tests/unit/banner.spec.ts). We use Cypress to check that when loading an image, if an error occurs the banner is not rendered.

For the tests we have modified the existing banner factory, this has caused some changes in other tests(mocked-responses.spec.ts).

@christianpe98 christianpe98 force-pushed the MOTPRD-10275-banner-more-flexible branch from 36f3923 to 64a04e3 Compare February 15, 2023 11:45
@christianpe98 christianpe98 marked this pull request as ready for review February 15, 2023 11:52
@christianpe98 christianpe98 requested a review from a team as a code owner February 15, 2023 11:52
@christianpe98 christianpe98 force-pushed the MOTPRD-10275-banner-more-flexible branch from 64a04e3 to e16f7ac Compare February 21, 2023 14:08
@christianpe98 christianpe98 force-pushed the MOTPRD-10275-banner-more-flexible branch from e16f7ac to e4feec3 Compare February 21, 2023 14:09
@tiborux tiborux merged commit 7207767 into main Feb 27, 2023
@tiborux tiborux deleted the MOTPRD-10275-banner-more-flexible branch February 27, 2023 08:19
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.

3 participants