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: Preventing Overflow #3217

Merged
merged 2 commits into from
Jun 28, 2019
Merged

fix: Preventing Overflow #3217

merged 2 commits into from
Jun 28, 2019

Conversation

kushthedude
Copy link
Member

@kushthedude kushthedude commented Jun 28, 2019

Fixes #3164

Changes proposed in this pull request:

  • Introducing Scrolling to disable Overflow caused by UI Modal

Checklist

  • I have read the Contribution & Best practices Guide.
  • My branch is up-to-date with the Upstream development branch.
  • The acceptance, integration, unit tests and linter pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@auto-label auto-label bot added the fix label Jun 28, 2019
@kushthedude
Copy link
Member Author

@shreyanshdwivedi @uds5501 Please Review !

@@ -49,56 +49,6 @@ export default ModalBase.extend(FormMixin, {
prompt : this.l10n.t('Please give us your tax ID')
}
]
},
taxInvoiceCompany: {
Copy link
Contributor

Choose a reason for hiding this comment

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

@kushthedude I don't think that the issue specified anywhere to remove the validations.

Copy link
Member Author

Choose a reason for hiding this comment

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

@uds5501 The validations which I removed were of optional fields of which tick box was recently removed, I dont think this fields should be mandatory unless and until event organiser is a firm.
@shreyanshdwivedi @mrsaicharan1 Your views ?

@kushthedude
Copy link
Member Author

@CosmicCoder96 @niranjan94 Please Review

Copy link
Member

@niranjan94 niranjan94 left a comment

Choose a reason for hiding this comment

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

@kushthedude when the issue is for fixing an overflow, then please fix only that in the PR. Remove all unrelated changes from this.

@kushthedude
Copy link
Member Author

@niranjan94 Removed all unwanted changes, Please Review

@niranjan94 niranjan94 changed the title fix: Preventing Overflow and Company Descriptions can be optional fix: Preventing Overflow Jun 28, 2019
@kushthedude
Copy link
Member Author

@CosmicCoder96 Review Please

Copy link
Member

@prateekj117 prateekj117 left a comment

Choose a reason for hiding this comment

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

LGTM

@abhinavk96 abhinavk96 merged commit 1a63370 into fossasia:development Jun 28, 2019
@kushthedude kushthedude deleted the css branch July 13, 2019 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Event Wizard: CSS body overflow in Tax Info Modal
5 participants