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

Initial Slots implementation #1563

Closed
wants to merge 10 commits into from
Closed

Initial Slots implementation #1563

wants to merge 10 commits into from

Conversation

sneridagh
Copy link
Member

No description provided.

- htmlHead
- htmlBeforeBody
- htmlAfterBody

Copy link
Member Author

Choose a reason for hiding this comment

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

I initially identified these slots (and their insertion positions), did I miss something? We can add more later, for sure. @nzambello the ones referring to your "Macros" PR are there as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think Sidebar is a good candidate, as well.

Though I can definitely see Martjin's point for the toolbar: if extra content is needed there, then maybe we need a system of definable actions. For the Toolbar, my wishlist include new buttons in the main toolbar, but also now components available for the "More" menu.

aboveContentTitle: [
// List of components (might have config too, maybe in `data` property)
{ path: '/', component: 'Component', data: {} },
// It can include blocks too (makes sense when we will be able to save them)
Copy link
Member Author

Choose a reason for hiding this comment

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

It's an initial thought about the future of slots, first iteration can have only "viewlet-ish", no sved config. In a second iteration we can provide a full backend support for saving those persistently, and have the ability to save slots state so we can even use blocks inside slots.

@sneridagh sneridagh changed the title Add initial documentation Initial Slots implementation May 30, 2020
@sneridagh sneridagh requested a review from tisto May 30, 2020 14:47
@sneridagh sneridagh linked an issue May 31, 2020 that may be closed by this pull request
Copy link
Contributor

@tiberiuichim tiberiuichim left a comment

Choose a reason for hiding this comment

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

Looks good, with one nitpick. I'd love to see a way to also override already registered slot components. This would avoid the need for such code (list items are harder to override then object items):

    const babelRuleIndex = config.module.rules.findIndex(
      rule =>
        rule.use &&
        rule.use[0].loader &&
        rule.use[0].loader.includes('babel-loader'),
    );

I propose that either:

  • each slot component also declares a name.
  • or we use a Map() instead of a list for the list of slot components.

@tiberiuichim tiberiuichim self-assigned this Aug 26, 2020
sneridagh and others added 3 commits October 30, 2020 09:07
* master: (414 commits)
  Update to use Node 12 or 14 (#1941)
  Back to development
  Release 8.7.1
  Prepare changelog for 8.7.1
  Fix German translation from -> E-Mail in contact form (#1955)
  Update issue templates (#1949)
  added loading icon when doing actions in folder-contents (#1950)
  Back to development
  Release 8.7.0
  Prepare for release
  Manage translations (Multilingual linking) view (#1944)
  Update issue templates (#1947)
  Force refresh of the docs css
  Update docs build and include pygments support for jsx (#1940)
  Fix tests and prettier (#1939)
  Back to development
  Release 8.6.0
  Preparing for release
  A pathname like /policy/edit does not show the Unauthorized component when not logged in (#1937)
  Render nativeLang as language selector (#1926)
  ...
@davisagli
Copy link
Member

@sneridagh Is there anything here worth keeping, or should we close the PR?

@sneridagh
Copy link
Member Author

Superseded by #5775

@sneridagh sneridagh closed this Mar 7, 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.

Slots in Volto
3 participants