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

Add permalinks for announcements #1054

Merged
merged 2 commits into from
Aug 11, 2019
Merged

Conversation

subins2000
Copy link

@subins2000 subins2000 commented Aug 10, 2019

Issue Reference

This PR addresses the Issue : Fixes #1017

Summarize

Add permalinks for announcements

Sample URL : https://keralarescue.in/announcements/2 https://keralarescue.in/announcements/4

Copy link
Member

@dauntlessnomad dauntlessnomad left a comment

Choose a reason for hiding this comment

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

@biswaz @vigneshhari please verify and merge

@sreenadh
Copy link

Why not a separate view file like announcement-view.html or announcements/view.html ?

@subins2000
Copy link
Author

subins2000 commented Aug 10, 2019

@sreenadh Because it's easier to manage the announcement card view in one page itself.

There's an additional proposal to add sharing buttons too.

@sreenadh
Copy link

Share buttons can be put in separate view file and include both in both views. In future we might need more feature separately for view and announcements then announcements.html will get bloated with if else conditions.

@subins2000
Copy link
Author

@sreenadh you're right. This was just a quick hack. In case there is more differences between the two, it should be separated.

@dauntlessnomad
Copy link
Member

fix the conflicts please

@dauntlessnomad dauntlessnomad self-requested a review August 11, 2019 09:53
@subins2000
Copy link
Author

@tomahawk-pilot done

@biswaz biswaz merged commit a624d95 into raksha-life:master Aug 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Give More Exposure to Announcements
4 participants