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

Implemented officers-page#198, yet again #253

Merged
merged 1 commit into from
Nov 2, 2021
Merged

Conversation

Joelrodiel
Copy link
Contributor

@Joelrodiel Joelrodiel commented Oct 11, 2021

I had previously implemented this feature in #205, but Jenkins decided it would have none of it. Now 4 months after, we've both grown a considerable amount, and have learned to put our differences aside for the good of the OCF and World Peace™.
This is the result:
image
This is addressing issue #198.

I created a json file in the assets folder with all the past officers data. I then parse that file in Officers.vue to construct it into a table inside a dropdown.

The good thing of this approach is that it's not hard coded and updating past officers will simply need an edit to the json file.

I look forward to pushing this merge together.

@bentref bentref self-requested a review October 12, 2021 03:15
Copy link
Contributor

@bentref bentref left a comment

Choose a reason for hiding this comment

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

'Twas quite a journey, so glad to see this ready to merge!

One suggestion: Would it make more sense to have each semester dropdown collapsed by default? Otherwise the dropdown functionality is unlikely to be ever used--you wouldn't go through and manually collapse a bunch

@bentref bentref merged commit 4d1afeb into master Nov 2, 2021
@bentref bentref deleted the officers-page#198 branch November 2, 2021 16:29
@bentref bentref linked an issue Nov 2, 2021 that may be closed by this pull request
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.

Complete Officers Page
2 participants