search

MarkBind / markbind

MarkBind is a tool for generating content-heavy websites from source files in Markdown format
https://markbind.org/
MIT License
134 stars 123 forks source link

Add the announcement component #2472

Closed Tim-Siu closed 2 months ago

Tim-Siu commented 3 months ago

What is the purpose of this pull request?

Overview of changes: Contribute to #2241 Related to #1960

This Pull request adds a announcement component. One usecase of the annoucement can be seen in the CS2103/T website.

Sample usage:

<announcement :dismissible="false" theme="warning" placement="bottom">
  <strong>This site is from a past semester!</strong> The current version will be <a href="http://www.comp.nus.edu.sg/~{{ module | lower }}">here</a> when the new semester starts.
</announcement>

<announcement :dismissible="true" theme="primary" placement="top-right">
  <strong>This site is from a past semester!</strong> The current version will be <a href="http://www.comp.nus.edu.sg/~{{ module | lower }}">here</a> when the new semester starts.
</announcement>

Output:

https://github.com/MarkBind/markbind/assets/61866948/11b1298c-4df2-4b78-b120-fd048c1cfcda

Parameters:

  1. dismissible: 'true', 'false;
  2. placement: 'top', 'top-right', 'top-left', 'bottom', 'bottom-right', 'bottom-left',
  3. theme: 'primary', 'secondary', 'success', 'warning', 'danger', 'info', 'light', 'dark',

Anything you'd like to highlight/discuss: Discussions: There are a lot of options we can add to this component.

Known limitations:

  1. Announcements may overlap each other
  2. Site wide application is not automatic

Testing instructions:

Proposed commit message: (wrap lines at 72 characters) Add an announcement component

Checklist: :ballot_box_with_check:

Reviewer checklist:

Indicate the SEMVER impact of the PR:

At the end of the review, please label the PR with the appropriate label: r.Major, r.Minor, r.Patch.

Breaking change release note preparation (if applicable):

Give a brief explanation note about:

  • what was the old feature that was made obsolete
  • any replacement feature (if any), and
  • how the author should modify his website to migrate from the old feature to the replacement feature (if possible).
codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 50.97%. Comparing base (4ab7e3a) to head (975657f).

Files Patch % Lines
packages/vue-components/src/Announcement.vue 0.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2472 +/- ## ========================================== - Coverage 50.98% 50.97% -0.02% ========================================== Files 124 125 +1 Lines 5305 5307 +2 Branches 1137 1137 ========================================== Hits 2705 2705 - Misses 2311 2313 +2 Partials 289 289 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

yucheng11122017 commented 3 months ago

@damithc, could we get your opinion on whether component is needed?

damithc commented 3 months ago

@damithc, could we get your opinion on whether component is needed?

@Tim-Siu thanks for taking this on. @yucheng11122017 thx for checking.

I quite like the placement/behavior I have in https://nus-cs2103-ay2021s1.github.io/website/ The only thing missing is a way to dismiss it. So, if there is a way to make such announcements easier to add and also dismissible, I think we can have it as a legit feature.

I prefer that to a toast that appears in front of other content and forces the user to dismiss it.

Tim-Siu commented 3 months ago

@damithc, could we get your opinion on whether component is needed?

@Tim-Siu thanks for taking this on. @yucheng11122017 thx for checking.

I quite like the placement/behavior I have in https://nus-cs2103-ay2021s1.github.io/website/ The only thing missing is a way to dismiss it. So, if there is a way to make such announcements easier to add and also dismissible, I think we can have it as a legit feature.

I prefer that to a toast that appears in front of other content and forces the user to dismiss it.

Sure. I will make the announcement component behave like that.

LamJiuFong commented 3 months ago

I am thinking of adding a minimise button next to the dismiss button, the only difference is by clicking the minimise button, the user can always re-click it (somewhere) to make the announcement appear again. Not sure if it is worth the effort implementing it. What do you think?

yucheng11122017 commented 3 months ago

Hi can you add documentation so its easier to test?

And also add test cases for this. Please do look through the checkpoint on the PR description to see what you should do for each PR

Tim-Siu commented 3 months ago

After discussions with Prof. Demith, it appears that extending Box components to support announcements will be a better approach.