[go: up one dir, main page]

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

enhance: macro rendered result has unnecessary right margin [breaking change] #9234

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

stdword
Copy link
Contributor
@stdword stdword commented Apr 25, 2023

I've added styles to leverage one-liner macros. Fixes #8623

before PR after PR

NOTE: This message is a summary and was updated. In the comments below you can find a history of all the changes during working on this PR.

Changes:

  1. macro alignment in one line (add .inline class)
  2. macro with headings left intact (no addition of .inline class)
  3. remove right margin (.mr-1)
  4. change styles for root div of hiccup & html syntax to «one-line» them too (add .inline class)

Test data

 :macros {
    "theme-test-macro0" "<b>macro</b>",
    "theme-test-macro1" "[:b \"macro\"]",
    "theme-test-macro2" "macro",
    "theme-test-macro3" "header\nmacro",
    
    "iframe" "<iframe src='$1' style='height:320px'></iframe>",
 }
- before {{theme-test-macro0}} after
- before {{theme-test-macro1}} after
- before {{theme-test-macro2}} after
- before {{theme-test-macro3}} after
- {{iframe https://wikipedia.com}}  ;; iframe should be as wide as Logseq block

Related discussions

⚠️ Caveats

  • This is my first clojure PR. I feel myself unsure on how I write this code. Please point me to issues if need be

@stdword stdword changed the title fix: macro rendered result has unnecessary right margin tiny fix: macro rendered result has unnecessary right margin Apr 25, 2023
@logseq-cldwalker
Copy link
Collaborator

Hi. I'm not able to repro the screenshot as a macro like {{ok foo}} | ok normally looks like
Screen Shot 2023-04-26 at 2 42 32 PM

Could you provide an example block and any custom.css you have that results in this case?

@stdword
Copy link
Contributor Author
stdword commented Apr 26, 2023

Whooh... you are right. No custom css and theme inductions (even cache was cleared) but there was a plugin that provide styles. Good point for me: plugins can change style too.

I've add .inline-block style to leverage one-liner macros in new commit.

So there are two changes here:

  1. macro alignment in one line
  2. remove right margin

WRONG: Note, macros with headers still look like without this changes:

UPDATED:

⚠️ I'll rework it to keep macros content with headers on the separate lines if need be and if you approve one-liners concept

 :macros {
    "theme-test-macro1" "[:b \"macro\"]",
    "theme-test-macro2" "macro",
    "theme-test-macro3" "header\nmacro",
 }
- before {{theme-test-macro1}} after
- before {{theme-test-macro2}} after
- before {{theme-test-macro3}} after

@logseq-cldwalker
Copy link
Collaborator

Thanks for the instructions.

macro alignment in one line

This is a breaking change. I don't use macros enough to have an opinion on this so will defer to others on the team

@stdword
Copy link
Contributor Author
stdword commented Apr 26, 2023

@logseq-cldwalker sure, thank you 🙏!

Please, consider this discussions:

@stdword
Copy link
Contributor Author
stdword commented May 3, 2023

@logseq-cldwalker, update PR to left default rendering behavior for macros with headings.

So, now it changes only one-liner macros.

@stdword
Copy link
Contributor Author
stdword commented May 7, 2023

Just found an issue I've fixed in this PR: #8623

@stdword
Copy link
Contributor Author
stdword commented May 9, 2023

Found additional case for iframe (described here). So I've changed the display style of macro-div (.inline instead of .inline-block). But Hiccup & HTML syntax inside macro gives an additional div block, so I've added styles for it to be inlined too.

Updated first post to keep all summary information about changes. This comment is for history reasons.

@logseq-cldwalker logseq-cldwalker changed the title tiny fix: macro rendered result has unnecessary right margin enhance: macro rendered result has unnecessary right margin [breaking change] Oct 16, 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.

Macros create unwanted newline/break
3 participants