Skip to content

Allow Download File Type dropdown to be deselected#1169

Merged
Nikhil-Nandagopal merged 1 commit into
appsmithorg:releasefrom
daniel-shuy:fix/download-file-type-dropdown-optional
Nov 5, 2020
Merged

Allow Download File Type dropdown to be deselected#1169
Nikhil-Nandagopal merged 1 commit into
appsmithorg:releasefrom
daniel-shuy:fix/download-file-type-dropdown-optional

Conversation

@daniel-shuy

@daniel-shuy daniel-shuy commented Oct 13, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Add a default selection to the Download File Type dropdown so that it can be deselected.
image

Fixes #674

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@daniel-shuy daniel-shuy force-pushed the fix/download-file-type-dropdown-optional branch from b1aeb12 to eb9cbfe Compare October 13, 2020 10:36
Nikhil-Nandagopal
Nikhil-Nandagopal reviewed Oct 13, 2020
View reviewed changes
Comment thread app/client/src/components/editorComponents/actioncreator/ActionCreator.tsx Outdated
Nikhil-Nandagopal
Nikhil-Nandagopal reviewed Oct 14, 2020
View reviewed changes
Comment thread app/client/src/components/editorComponents/actioncreator/ActionCreator.tsx Outdated

@Nikhil-Nandagopal Nikhil-Nandagopal Oct 14, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@daniel-shuy for some reason this option is always showing selected even when it's not. Might have to have a valid id here or it could be something else

@daniel-shuy daniel-shuy Oct 14, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Turns out that was due to the undefined value, which was causing a compilation error (TypeScript strict mode is enabled). Changing the value back to empty string solves it

@daniel-shuy daniel-shuy Oct 14, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've dropped the previous commit

@Nikhil-Nandagopal Nikhil-Nandagopal Oct 14, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok this looks good then. Can you check why the tests are failing? Likely pull release into your branch should solve this

@daniel-shuy daniel-shuy Oct 14, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ironically it started failing after I rebased onto release. #1191 should fix it I think.

@Nikhil-Nandagopal Nikhil-Nandagopal Oct 14, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yikes! Sorry about that 😅

Nikhil-Nandagopal
Nikhil-Nandagopal requested changes Oct 14, 2020
View reviewed changes

@Nikhil-Nandagopal Nikhil-Nandagopal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requested changes

@daniel-shuy daniel-shuy force-pushed the fix/download-file-type-dropdown-optional branch from 377588f to eb9cbfe Compare October 14, 2020 07:25
@daniel-shuy daniel-shuy force-pushed the fix/download-file-type-dropdown-optional branch from eadce46 to ffb510f Compare October 14, 2020 08:37
@daniel-shuy daniel-shuy force-pushed the fix/download-file-type-dropdown-optional branch from ffb510f to 1c6cf92 Compare October 22, 2020 13:40
@daniel-shuy daniel-shuy force-pushed the fix/download-file-type-dropdown-optional branch from 1c6cf92 to d3fdda5 Compare October 31, 2020 14:05
@daniel-shuy

daniel-shuy commented Oct 31, 2020

Copy link
Copy Markdown
Contributor Author

Rebased onto release, hopefully the tests are fixed

@mohanarpit

mohanarpit commented Nov 3, 2020

Copy link
Copy Markdown
Member

@daniel-shuy I'm really sorry about this but can you please update your branch again with the latest code from release? There were some breaking changes introduced in release which is probably causing tests to fail on your branch. We've fixed those tests and the CI code as well.

The tests should then pass on this branch.

@daniel-shuy daniel-shuy force-pushed the fix/download-file-type-dropdown-optional branch from d3fdda5 to 2a0dfd1 Compare November 4, 2020 04:14
@daniel-shuy daniel-shuy force-pushed the fix/download-file-type-dropdown-optional branch from 2a0dfd1 to 2c5e743 Compare November 4, 2020 12:24
@daniel-shuy

daniel-shuy commented Nov 4, 2020

Copy link
Copy Markdown
Contributor Author

@mohanarpit it's passing now, thanks!

@Nikhil-Nandagopal Nikhil-Nandagopal merged commit a110a23 into appsmithorg:release Nov 5, 2020
@daniel-shuy daniel-shuy deleted the fix/download-file-type-dropdown-optional branch November 6, 2020 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Nikhil-Nandagopal Nikhil-Nandagopal Nikhil-Nandagopal approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] 'type' field once selected, no more remains an optional field.

3 participants

@daniel-shuy @mohanarpit @Nikhil-Nandagopal