Skip to content

[fix] account for various cases of orphaned dividers #22

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kantorge
Copy link
Contributor

There are some cases, when the empty header results in orphaned dividers still being rendered on the top of the list. This can happen when a conditional block is also added to the top of the list. The proposed changes account for this situation.

More specifically, the PR contains the following changes:

  • The "remove duplicate dividers" code is moved first in the divider cleanup block. This ensures that such duplicates are cleaned up at any position.
  • The orphan check is added for the beginning of the list.
  • I've added a test HTML file which is not using the header for easy review of such cases.
  • I've extracted the sample data to have a common sample set for all tests.

The main change about the divider cleanup can be best tested selecting employees from Tokyo or from any other city.

Please review the PR, and merge it if you find it useful. Let me know, if you have any questions or comments.

@torrobinson
Copy link
Owner

torrobinson commented Mar 28, 2024

Thanks I appreciate the PR! Happy to hear that people are still using this and have enhancements (I've since changed jobs and moved away from this tech stack - I actually originally wrote this extension at home so I could use it at work in a certain project lol). Give me a few days to take a peek at it, as I've been busy and haven't had a chance to look yet 😄

@torrobinson torrobinson self-requested a review March 28, 2024 16:29
@torrobinson torrobinson self-assigned this Mar 28, 2024
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.

2 participants