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

[docs] Fix link icons for file reference section titles #3709

Merged
merged 1 commit into from
Jun 24, 2024

Conversation

Janpot
Copy link
Member

@Janpot Janpot commented Jun 24, 2024

Fix file reference doc:

Screenshot 2024-06-24 at 11 52 38

preview of the fix.

The SectionTitle component is a step in the good direction, but it still requires a global setup in the consuming application. it needs the sprite present on the page:

<svg style={{ display: 'none' }} xmlns="http:https://www.w3.org/2000/svg">
  <symbol id="anchor-link-icon" viewBox="0 0 16 16">
    <path d="M4 9h1v1H4c-1.5 0-3-1.69-3-3.5S2.55 3 4 3h4c1.45 0 3 1.69 3 3.5 0 1.41-.91 2.72-2 3.25V8.59c.58-.45 1-1.27 1-2.09C10 5.22 8.98 4 8 4H4c-.98 0-2 1.22-2 2.5S3 9 4 9zm9-3h-1v1h1c1 0 2 1.22 2 2.5S13.98 12 13 12H9c-.98 0-2-1.22-2-2.5 0-.83.42-1.64 1-2.09V6.25c-1.09.53-2 1.84-2 3.25C6 11.31 7.55 13 9 13h4c1.45 0 3-1.69 3-3.5S14.5 6 13 6z" />
  </symbol>
</svg>

Is there a good way to make this component fully self contained?

@Janpot Janpot added the docs Improvements or additions to the documentation label Jun 24, 2024
@Janpot Janpot requested a review from a team June 24, 2024 09:54
@Janpot Janpot changed the title [docs] Fix file reference section title icons [docs] Fix icons for file reference section titles Jun 24, 2024
@Janpot Janpot changed the title [docs] Fix icons for file reference section titles [docs] Fix link icons for file reference section titles Jun 24, 2024
@Janpot Janpot marked this pull request as ready for review June 24, 2024 10:05
Copy link
Member

@alexfauquette alexfauquette left a comment

Choose a reason for hiding this comment

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

I don't see how to manage that in the SectionTitle file and at the same time be sure we do not add the <svg> component multiple time in a single page.

The best I can think about is a

function LayoutIcons({ icons , children }) {
  const defaultisedIdons = { ...defaultIcons, ...icons }
  return (<>
    {children}
    {Object.entries(([iconId, icon]) => 
      <svg key={iconId} style={{ display: 'none' }} xmlns="http:https://www.w3.org/2000/svg">
        <symbol id={iconId} viewBox="0 0 16 16" />
        {icon}
      </svg>
   )}
  </>);
}

@Janpot Janpot merged commit 94d37ae into mui:master Jun 24, 2024
13 of 14 checks passed
@Janpot Janpot deleted the ref-sections branch June 24, 2024 14:13
@Janpot
Copy link
Member Author

Janpot commented Jun 24, 2024

We could create the sprite imperatively, but not convinced that would be more maintainable 🙂

function SectionTitle() {
  React.useEffect(() => {
    let sprite = document.getElementById('svg-sprite');
    if (!sprite) {
      sprite = document.createElement('svg');
      sprite.outerHTML = `<svg id="svg-sprite" style="display:none" xmlns="http:https://www.w3.org/2000/svg"></svg>`;
      document.appendChild(sprite);
    }
    let symbol = document.getElementById('section-title-symbol');
    if (!symbol) {
      symbol = document.createElement('symbol');
      symbol.outerHTML = `<symbol id="anchor-link-icon" viewBox="0 0 16 16"><path d="M4 9h1v1H4c-1.5 0-3-1.69-3-3.5S2.55 3 4 3h4c1.45 0 3 1.69 3 3.5 0 1.41-.91 2.72-2 3.25V8.59c.58-.45 1-1.27 1-2.09C10 5.22 8.98 4 8 4H4c-.98 0-2 1.22-2 2.5S3 9 4 9zm9-3h-1v1h1c1 0 2 1.22 2 2.5S13.98 12 13 12H9c-.98 0-2-1.22-2-2.5 0-.83.42-1.64 1-2.09V6.25c-1.09.53-2 1.84-2 3.25C6 11.31 7.55 13 9 13h4c1.45 0 3-1.69 3-3.5S14.5 6 13 6z" /></symbol>`;
      sprite.appendChild(symbol);
    }
  }, []);
  return <div>Render section title here</div>;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants