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

fix: Bug fixed for add filter button in production #4284

Merged
merged 10 commits into from
Oct 15, 2023

Conversation

sjcode99
Copy link
Contributor

@sjcode99 sjcode99 commented Oct 1, 2023

What change does this PR introduce?

This PR fixes the bug for the Add Filter button in production

The change was to enable the add filter button in the production environment.

Fixes #4254

Copy link
Contributor

@LetItRock LetItRock left a comment

Choose a reason for hiding this comment

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

@sjcode99 left you a comment in the original ticket ;)

@sjcode99
Copy link
Contributor Author

sjcode99 commented Oct 2, 2023

@LetItRock updated the PR with the latest changes. Please verify it . Thanks

Copy link
Contributor

@LetItRock LetItRock left a comment

Choose a reason for hiding this comment

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

hey @sjcode99 👋
almost there, you are doing great! 🙌 just noticed that we didn't cover all the filter cases, please take a look at the below screenshot ;)
Screenshot 2023-10-02 at 17 45 53

@sjcode99
Copy link
Contributor Author

sjcode99 commented Oct 2, 2023

@LetItRock, Ohh!!😮 I have missed this. Changes update, Please have a look.

Copy link
Contributor

@scopsy scopsy left a comment

Choose a reason for hiding this comment

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

Thank you @sjcode99 looking good! Just a small thing here to fix this un-needed import

@@ -9,6 +9,7 @@ import { DeleteStepButton, FilterButton } from './FilterModal.styles';
import { OnlineFiltersForms } from './OnlineFiltersForms';
import { PreviousStepFiltersForm } from './PreviousStepFiltersForm';
import { useMemo } from 'react';
import { boolean } from 'zod';
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like it was imported by a mistake.

@@ -44,7 +44,7 @@ export function StepSettings({ index }: { index: number }) {
onClick={() => {
setFilterOpen(true);
}}
disabled={readonly}
disabled={false}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
disabled={false}

@sjcode99
Copy link
Contributor Author

sjcode99 commented Oct 3, 2023

Hey @scopsy, removed the unnecessary import. Please have a look

@sjcode99
Copy link
Contributor Author

sjcode99 commented Oct 5, 2023

Hey @LetItRock @scopsy . Seems my code is not yet merged. can you please help with this merge

@sjcode99
Copy link
Contributor Author

Hey @LetItRock, it's been a long since I created PR and I can still see it's not merged. Is there any issue in the code which needs to be addressed?
Thanks :)

@ainouzgali ainouzgali merged commit c9d0667 into novuhq:next Oct 15, 2023
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[NV-2350] Should show filter modal in production
4 participants