zhouhua / obsidian-sticky-headings

MIT License
22 stars 1 forks source link

Feature request: add toggle or remove automatic padding below heading list. #20

Closed jmonroynieto closed 2 months ago

jmonroynieto commented 2 months ago

I was trying to reduce the amount of padding between a stylized heading list and found that the value is programmatically set causing the value to shift as you scroll. I find it a bit distracting and would like to put in a set value.

image1

Using the developer tools, I enabled a "break on attribute change" for the padding value of interest. The line causing the issue is https://github.com/zhouhua/obsidian-sticky-headings/blob/7f103667e684a3b0258ab40c4bc761fd7acd4e48/src/Plugin.ts#L241

I don't know about other users, therefore the least disruptive implementation of a change would be to have a setting for it in the plugin configuration allowing us to disable automatic recalculation. It could be a value capture; when set to 0, people retain the automatic padding. Otherwise, the value is set in pixels.

jmonroynieto commented 2 months ago

This is my CSS rule to replicate the above style changes:

.sticky-headings-root {
      transition: height 0.25s ease-in-out;
    background-color: #101818;
    border-radius: 12px;
    width: 40%;
    margin: auto;
    left: 0px;
    right: 0px;
    position: inherit;
    padding-top: 3px;
    padding-bottom: 4px;
}
itsonlyjames commented 2 months ago

@jmonroynieto this is an issue in parallel with one I've just created: https://github.com/zhouhua/obsidian-sticky-headings/issues/21

I'm not entirely sure the reasoning for adding the padding, I will investigate when I get some time however. I don't think the elements get removed from the dom by this plugin so I'm not sure immediately why the padding is necessary—plus the sticky container is absolute and not part of the documents flow anyhow.

Updates to come.

zhouhua commented 2 months ago

I'm not entirely sure the reasoning for adding the padding

@itsonlyjames Setting a padding-top for the editor that matches the height of .sticky-headings-root is to ensure more accurate calculations when the title is about to scroll out of the visible area.

In the scenario provided by @jmonroynieto, .sticky-headings-root appears to "float" above the editor (btw, this UI looks great), which is different from our default that .sticky-headings-root overlays the editor. I don't fully understand how you intend to solve this issue yet, but my initial thought is that we might need to add a setting option specifically for this type of UI.

jmonroynieto commented 2 months ago

I hope the changes resolve this. Other issues include the list being clipped sometimes due to the height modulator, especially when performing a search.

image

https://github.com/zhouhua/obsidian-sticky-headings/blob/7f103667e684a3b0258ab40c4bc761fd7acd4e48/src/Plugin.ts#L237

itsonlyjames commented 2 months ago

@jmonroynieto this issue is likely to still be apparent and will not be fixed by the scrolling issue due to the document flow. I also noticed this yesterday when I had 5 stacked headers and the search bar was below the heading element and moved around while searching. We will need to come up with a different solution for this. My initial thoughts are overlaying the search on top of the sticky header element, but it may not look the nicest. Do you have any ideas?

Second to this, I am going to move the background styling to the inner element so the sides don't get cut off like in your screenshots, we may need to collaborate on some CSS to get it working perfect 👌

jmonroynieto commented 2 months ago

I actually think that document-search-container should go right after the titlebar but this is not possible due to being bundled inside the mardowneditor div. This would also impede the "floating" solution that you are suggesting. I am not a JS coder so I am commenting under the assumption that breaking up the large container for all editor divs is not possible. Beyond pointing to the lines that I found, I have no other insights into possible implementations.

I don't know if I understand your second point. The CSS styling of the flanks is perfect as it is. The sides are reduced intentionally for a "pill" style. The inner element "sticky-headings-container" would not allow this.

itsonlyjames commented 2 months ago

Where the search element is it will always be "under" the sticky header, but where it is makes it at the "top" of the document.

image

Is your theme custom or is it downloadable so I can test it out?

jmonroynieto commented 2 months ago

Thank you for the explanation, floating sticky element would be great.

My theme is just a few css snippets. The only piece pertaining to this issue is the styling for the heading list. I included my current rule above to replicate.

I can also gladly push it into a repo for your tests.

jmonroynieto commented 2 months ago

Upcoming changes are tackling the solution to the problems outlined in this issue. While we get a proper look at the plugin's upcoming performance, I'm closing this issue.