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

feature: refactor and add banner to storybook #1467

Merged
merged 2 commits into from
Jul 25, 2024

Conversation

knav33n
Copy link
Contributor

@knav33n knav33n commented Jul 19, 2024

Problem

1243

Changes Made

  • Refactored Banner into a generic component and added it to Storybook.
  • Created GithubBanner by extending the generic Banner component.

Screenshots

banner-storybook

Checklist

  • Code follows the contributing guidelines
  • Changes have been thoroughly tested
  • Documentation has been updated, if necessary
  • Any dependencies have been added to the project, if necessary

Reviewers

@Traxmaxx @Azanul @AllieMendes @mlabouardy

Copy link
Collaborator

@AvineshTripathi AvineshTripathi left a comment

Choose a reason for hiding this comment

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

@Azanul to do detail review

@@ -35,7 +35,7 @@ function useGithubStarBanner() {
useEffect(() => {
const shouldDisplayBanner = checkLocalStorageForBannerStatus();

if (shouldDisplayBanner !== 'false' && router.query.view) {
if (true) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why are we making this true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My apologies – I used this for some last-minute testing.

Copy link
Collaborator

@AvineshTripathi AvineshTripathi left a comment

Choose a reason for hiding this comment

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

LGTM

@Azanul please review the frontend changes! I saw the working and overview looks good to me

@Azanul
Copy link
Collaborator

Azanul commented Jul 24, 2024

@AvineshTripathi Please approve the PR if everything looks

@Azanul Azanul merged commit 2832ddb into tailwarden:develop Jul 25, 2024
3 checks passed
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