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

Polishing: Some design issues #2744 #2759

Merged
merged 7 commits into from
Feb 15, 2023

Conversation

nikitagrossman
Copy link
Contributor

@nikitagrossman nikitagrossman commented Feb 14, 2023

[NV-1657] ✨ Polishing: Some design issues in the Integration Store #2744

What change does this PR introduce?

minor design changes in integration store

buttons do not scroll when inside form + added padding so it looks much better & when u hover on a !active provider card the opacity comes up to 1

Why was this change needed?

better looking

closes #2744
closes #2388

Screen.Recording.2023-02-14.at.11.37.09.mov

[NV-1657] ✨ Polishing: Some design issues in the Integration Store novuhq#2744
@@ -1,4 +1,23 @@
import { defineConfig } from 'cypress';
const configOverride = require('./config-overrides');
Copy link
Contributor

Choose a reason for hiding this comment

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

This was needed to make cypress work with babelrc

@@ -311,6 +311,8 @@ describe('Creation functionality', function () {
cy.getByTestId('groupSelector').clear();
cy.getByTestId('groupSelector').type('New Test Category');
cy.getByTestId('submit-category-btn').click();
cy.waitForNetworkIdle(500);
Copy link
Contributor

Choose a reason for hiding this comment

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

Flaky test here before, so hope it will help with it

@@ -350,6 +350,7 @@ const InputWrapper = styled.div`
margin-bottom: 10px;
font-size: 14px;
}
padding-right: 30px;
Copy link
Contributor

Choose a reason for hiding this comment

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

@nikitagrossman why is the padding here needed?

If any, I would put it on the CenterDiv component instead of on the input wrapppers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i did that so the scroll bar would not touch any inputs
anyway i changed that

fix: padding moved to centerDiv
Copy link
Contributor

@BiswaViraj BiswaViraj left a comment

Choose a reason for hiding this comment

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

🚀

@scopsy scopsy added this pull request to the merge queue Feb 15, 2023
@scopsy
Copy link
Contributor

scopsy commented Feb 15, 2023

Noice!

Merged via the queue into novuhq:next with commit 0a6a81c Feb 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants