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

feat: add external event url menu option in EventDetailsFragment #1508

Merged
merged 1 commit into from
Apr 4, 2019

Conversation

addiegupta
Copy link
Contributor

@addiegupta addiegupta commented Apr 2, 2019

Fixes: #1505

Changes:

  • Added a menu option to EventDetailsFragment to navigate to the external event url for the event
  • If the external URL is null, then the menu option is disabled

Screenshots for the change:

prfor1505

Additional details:

  • The option has been titled 'Open External Event URL' to maintain consistency with the eventyay frontend e.g. check the option on the right side at https://eventyay.com/e/a1fcaa21/coc/

  • Instead of removing the menu item if the URL is null, I have chosen to disable the option because removing the item requires calling invalidateOptionsMenu() which will force the menu options to be prepared once again. This resets the favorite icon status also hence the favorite icon will have to be set again. Hence to avoid this complexity I have simply disabled it. If suggested, I can use this approach to remove the option altogether when the URL is null

EDIT: Updated GIF after now hiding option when URL is null
Will squash commits once this gets approved

prfor1505new

@liveHarshit
Copy link
Member

  • can use this approach to remove the option altogether when the URL is null

IMO, hiding it is a better option.

@addiegupta
Copy link
Contributor Author

@liveHarshit changes made

Copy link
Member

@liveHarshit liveHarshit left a comment

Choose a reason for hiding this comment

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

LGTM

Fixes: fossasia#1505

Changes:

- Added a menu option to EventDetailsFragment to navigate to the external event url for the event
- If the external URL is null, then the menu option is hidden

Additional Changes:
- Modified the methods used to mark favorite icon to be consistent with above added features
@addiegupta
Copy link
Contributor Author

@liveHarshit removed extra lines and squashed commits

if (eventShare.externalEventUrl == null) {
menu.findItem(R.id.open_external_event_url).isVisible = false
}
setFavoriteIconFilled(eventShare.favorite)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nikit19 this statement is also to be executed only if eventShare has been initialised

@iamareebjamal iamareebjamal added this to the v0.2.0 milestone Apr 4, 2019
@iamareebjamal iamareebjamal merged commit 226ad66 into fossasia:development Apr 4, 2019
@addiegupta addiegupta deleted the 1505 branch April 4, 2019 08:10
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.

4 participants