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 horizontal spacing between the icon and text in the custom icon list #2352 #2375

Merged
merged 7 commits into from Mar 12, 2024

Conversation

Ashleeey1224
Copy link
Contributor

@Ashleeey1224 Ashleeey1224 commented Oct 28, 2023

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • DevOps
  • Improve developer experience
  • Others, please explain:

Resolves #2352
This PR resolves issue #2352 by adding i-spacing attribute for modifying horizontal spacing between the icon and text in the custom icon list.

Overview of changes:

  1. By defining "{i-spacing="xem"}" attribute, user can customise the horizontal distance between icon and text. x means numbers.
  • CODE:

* **Item 1**: i-spacing is 0.15em {icon="glyphicon-education" i-width="70px" i-spacing="0.15em"}
* **Item 2**: i-spacing is 0.55em{i-width="70px" i-spacing="0.55em"}
* Item 2.1: no i-spacing attribute, default is 0.35em {icon="fas-question-circle"}
* Item 2.2:
* Item 2.3: overwrite i-spacing attribute by 2em {i-spacing="2em"}
* **Item 3**{i-width="70px"}
* Item 3.1: {icon="glyphicon-book" i-width="70px"}
* Item 3.2: {icon="/images/AzurLane_roma copy.png" i-size="10px" i-class="rounded"}

  • OUTPUT:

Screenshot 2023-10-29 at 12 02 50 am

Anything you'd like to highlight/discuss:

Once a i-spacing attribute is assigned to an icon, it will continue apply to icons at the same level (e.g., Item2.3, Item3.1, Item3.2), until it is overridden by a different i-spacing attribute (e.g., from Item 2.1 to Item 2.3).

Testing instructions:

  1. Confirm that you have the correct branch for this pull request checked out on your local environment.
  2. Establish a new MarkBind project using the markbind init command.
  3. Experiment with the list using the provided syntax from above.

Proposed commit message: (wrap lines at 72 characters)
Add i-spacing attribute to custom icon list


Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

- Allow users to modify icon's size, width, height, and class for customizing icon attributes while also providing the option to adjust spacing between the icon and text for enhanced user experience.

- Add "i-spacing" attribute to ICON_ATTRIBUTES and interface IconAttributes in CustomListIconProcessor.ts.

- Implement logic to check whether the user has defined the i-spacing attribute or not.

- Update createIconSpan function to incorporate i-spacing attribute.

- Update getIconAttributes function to retrieve i-spacing attribute.

- This consolidation of attributes maintains code consistency and cleanliness.

- Refer issues: MarkBind#2352, MarkBind#2316.
@Ashleeey1224
Copy link
Contributor Author

feel free to leave some feedback:) happy to review and improve it

@codecov
Copy link

codecov bot commented Oct 30, 2023

Codecov Report

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

Project coverage is 48.90%. Comparing base (1c01cfb) to head (624d093).

Files Patch % Lines
packages/core/src/html/CustomListIconProcessor.ts 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2375      +/-   ##
==========================================
- Coverage   48.92%   48.90%   -0.02%     
==========================================
  Files         124      124              
  Lines        5245     5247       +2     
  Branches     1110     1112       +2     
==========================================
  Hits         2566     2566              
- Misses       2371     2373       +2     
  Partials      308      308              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tlylt
Copy link
Contributor

tlylt commented Nov 13, 2023

Hi @Ashleeey1224, sorry for the late response. Please wait for the discussion in the original issue to conclude before making further changes.

@tlylt
Copy link
Contributor

tlylt commented Dec 10, 2023

I will close this for now as a conclusion is not yet reached in the original issue. Please feel free to re-open after the investigation/discussion is completed.

@tlylt tlylt closed this Dec 10, 2023
@yucheng11122017 yucheng11122017 marked this pull request as ready for review March 8, 2024 13:23
Copy link
Contributor

@EltonGohJH EltonGohJH left a comment

Choose a reason for hiding this comment

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

Overall LGTM.
I think just 1 small nit and also the potential repositioning of a part of the docs!

@yucheng11122017 yucheng11122017 mentioned this pull request Mar 11, 2024
14 tasks
@tlylt
Copy link
Contributor

tlylt commented Mar 11, 2024

@yucheng11122017 is this PR still linked to #2352? Need to update the description to target it properly.

Copy link
Contributor

@kaixin-hc kaixin-hc left a comment

Choose a reason for hiding this comment

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

lgtm

@yucheng11122017 yucheng11122017 merged commit 6370008 into MarkBind:master Mar 12, 2024
8 of 10 checks passed
@yucheng11122017
Copy link
Contributor

@all-contributors please add @Ashleeey1224 for code

Copy link
Contributor

@yucheng11122017

I've put up a pull request to add @Ashleeey1224! 🎉

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.

Add horizontal spacing between the icon and text in the custom icon list
5 participants