Skip to content

Conversation

@uds5501
Copy link
Contributor

@uds5501 uds5501 commented Jun 13, 2019

Fixes #3136

Changes proposed in this pull request:

  • Add component level logic for hiding/showing filters
  • Toggle filters in small devices.

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)

Two mobile views of the implementation, with and without the filters

Without :

Screenshot from 2019-06-13 18-18-44

With :

Screenshot from 2019-06-13 18-18-49

@auto-label auto-label bot added the feature label Jun 13, 2019
return getDateRanges.bind(this)();
}),

showFiltersOnMobile: computed('device.isMobile', 'showFilters', function() {
Copy link
Member

Choose a reason for hiding this comment

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

Use computed.and instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually had to change the logic because the filters weren't showing up on desktop.. Please review now? @niranjan94

@uds5501 uds5501 force-pushed the explore-browser-section branch from 6cb7da2 to 8a28670 Compare June 13, 2019 17:14
}),

showFiltersOnMobile: computed('device.isMobile', 'showFilters', function() {
return ((this.device.isMobile && this.showFilters) || !this.device.isMobile);
Copy link
Member

Choose a reason for hiding this comment

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

@uds5501 could this logic be simplified into

!this.device.isMobile || this.showFilters

@uds5501 uds5501 changed the title feat: remove filters in mobile devices and add option to hide/unhide map feat: toggle filters in mobile devices = Jun 14, 2019
@uds5501 uds5501 changed the title feat: toggle filters in mobile devices = feat: toggle filters in mobile devices Jun 14, 2019
Copy link
Member

@kushthedude kushthedude left a comment

Choose a reason for hiding this comment

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

@uds5501 A toggle switch would be better than a button

@uds5501
Copy link
Contributor Author

uds5501 commented Jun 14, 2019

@kushthedude I think I tried following EventBrite UI as closely as possible without disturbing the basic UI of the code, they went with button so I think it's best we go with button too 😅 .

Of course, I am prepared to make the change if the mentors agree to that. @CosmicCoder96 @niranjan94 @SaptakS @pradeepgangwar

@niranjan94 niranjan94 merged commit 2a7fab7 into fossasia:development Jun 15, 2019
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.

Filter removal from explore page in mobile devices

4 participants