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.

Sign up for GitHub

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

Jump to bottom

Add configuration to create and use global block on a namespace #1727

Open
wants to merge 14 commits into
base: 2024.9.x
Choose a base branch
from
Open

Add configuration to create and use global block on a namespace #1727

wants to merge 14 commits into from

Conversation

kelanik8
Copy link
Contributor

@kelanik8 kelanik8 commented Mar 7, 2024

The following changes are implemented

TODO: Summary

Changes in the user interface:

TODO: Add screenshots, recordings or remove this section

Checklist when submitting a final (!draft) PR

  • Commits are tidied up, squashed if needed and follow guidelines in CONTRIBUTING.md
  • Code builds
  • All existing tests pass
  • All new critical code is covered by tests
  • PR is linked to the relevant issue(s)
  • Rebased with the target branch

@kelanik8 kelanik8 requested a review from Fajfa March 7, 2024 14:45
@kelanik8 kelanik8 force-pushed the 2024.3.x-feature-global-blocks branch 2 times, most recently from 4fc9322 to decffff Compare March 8, 2024 08:54
@kelanik8 kelanik8 force-pushed the 2024.3.x-feature-global-blocks branch 4 times, most recently from e1eb578 to edd2604 Compare March 8, 2024 11:53
@kelanik8 kelanik8 linked an issue Mar 8, 2024 that may be closed by this pull request
Add option to make and use global blocks #1598
Open
@kelanik8 kelanik8 changed the title Add functionality to create and use global block on a namespace Add configuration to create and use global block on a namespace Mar 11, 2024
@kelanik8 kelanik8 force-pushed the 2024.3.x-feature-global-blocks branch from edd2604 to f3d4d96 Compare March 11, 2024 11:14
Fajfa
Fajfa requested changes Mar 11, 2024
View reviewed changes
client/web/compose/src/components/Admin/Page/Builder/Selector.vue Outdated Show resolved Hide resolved
client/web/compose/src/components/Admin/Page/Builder/Selector.vue Outdated Show resolved Hide resolved
client/web/compose/src/components/Admin/Page/Builder/Selector.vue Outdated Show resolved Hide resolved
client/web/compose/src/components/Admin/Page/Builder/Selector.vue Outdated Show resolved Hide resolved
client/web/compose/src/components/Admin/Page/Builder/Selector.vue Outdated Show resolved Hide resolved
client/web/compose/src/views/Admin/Pages/Builder.vue Outdated Show resolved Hide resolved
client/web/compose/src/views/Admin/Pages/Builder.vue Outdated Show resolved Hide resolved
lib/js/src/compose/types/page-block/base.ts Outdated Show resolved Hide resolved
lib/js/src/compose/types/page-block/base.ts Outdated Show resolved Hide resolved
locale/en/corteza-webapp-compose/block.yaml Outdated Show resolved Hide resolved
@kelanik8 kelanik8 force-pushed the 2024.3.x-feature-global-blocks branch from baae52b to 4ec6a59 Compare March 12, 2024 14:50
Fajfa
Fajfa requested changes Mar 15, 2024
View reviewed changes
client/web/compose/src/views/Admin/Pages/Builder.vue Outdated Show resolved Hide resolved
lib/js/src/compose/types/page-block/base.ts Outdated Show resolved Hide resolved
@katrinDY katrinDY requested a review from tjerman March 19, 2024 09:37
tjerman
tjerman requested changes Mar 19, 2024
View reviewed changes
Copy link
Member

@tjerman tjerman left a comment

Choose a reason for hiding this comment

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

In addition to this; what happens if we change the block definition (the name/id for example), delete it, etc? How do we assure references are ok and we don't just end up with broken pages?

server/compose/envoy/yaml_decode.go Show resolved Hide resolved
server/compose/envoy/yaml_encode.go Outdated Show resolved Hide resolved
server/compose/types/namespace.go Outdated Show resolved Hide resolved
server/store/adapters/rdbms/upgrade_fixes.go Outdated Show resolved Hide resolved
@Fajfa Fajfa force-pushed the 2024.3.x branch 2 times, most recently from 125e8c4 to 6198193 Compare March 22, 2024 13:54
tjerman
tjerman approved these changes Mar 26, 2024
View reviewed changes
Copy link
Member

@tjerman tjerman left a comment

Choose a reason for hiding this comment

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

A consideration, else looks good to me 👍 ; wp

server/compose/types/namespace.go
Comment on lines +34 to +52
GlobalBlock struct {
BlockID uint64 `json:"blockID,string,omitempty"`

Options map[string]interface{} `json:"options,omitempty"`
Style PageBlockStyle `json:"style,omitempty"`
Kind string `json:"kind"`
XYWH [4]int `json:"xywh"` // x,y,w,h
Meta map[string]any `json:"meta,omitempty"`

// Warning: value of this field is now handled via resource-translation facility
// struct field is kept for the convenience for now since it allows us
// easy encoding/decoding of the outgoing/incoming values
Title string `json:"title,omitempty"`

// Warning: value of this field is now handled via resource-translation facility
// struct field is kept for the convenience for now since it allows us
// easy encoding/decoding of the outgoing/incoming values
Description string `json:"description,omitempty"`
}
Copy link
Member

Choose a reason for hiding this comment

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

Seems fine but you could consider just doing this:

GlobalBlock struct {
		PageBlock
	}

Since you're basically doing a full blown copy of the page block this would be fine.
For page layouts, I preserve just the reference and coordinates and size since that's all that we care about there.

@kelanik8 kelanik8 force-pushed the 2024.3.x-feature-global-blocks branch from 64d5780 to 7bc30f3 Compare April 8, 2024 12:48
@katrinDY
Copy link
Contributor

katrinDY commented Apr 15, 2024 •
edited
Loading

A couple of questions:

  • is the margin-top here intentional?
Screenshot 2024-04-15 at 10 46 43
  • after marking a block as global and copying it without reference, the new block isn't marked as global. Is this intentional? confirmed; it's fine
  • after marking a block as global and copying it with reference, the new block is marked as global. Is this intentional? confirmed; it's fine

@kelanik8 kelanik8 force-pushed the 2024.3.x-feature-global-blocks branch from fce0ad6 to 57e6a7f Compare April 16, 2024 11:03
@Fajfa Fajfa force-pushed the 2024.3.x branch 2 times, most recently from d2fcfdf to bf68606 Compare April 29, 2024 13:33
@Fajfa Fajfa force-pushed the 2024.3.x branch 2 times, most recently from d04c433 to 4b18cc5 Compare May 21, 2024 12:44
@github-actions GitHub Actions
Copy link

Stale pull request message

@Fajfa Fajfa deleted the branch 2024.9.x September 30, 2024 11:27
@Fajfa Fajfa closed this Sep 30, 2024
@Fajfa Fajfa reopened this Sep 30, 2024
@Fajfa Fajfa changed the base branch from 2024.3.x to 2024.9.x September 30, 2024 11:34
@Fajfa Fajfa force-pushed the 2024.9.x branch 2 times, most recently from d5eef63 to a48155e Compare October 8, 2024 13:07
@Fajfa Fajfa force-pushed the 2024.9.x branch 3 times, most recently from 16027e5 to 63a0a66 Compare October 16, 2024 12:07
@Fajfa Fajfa force-pushed the 2024.9.x branch 3 times, most recently from b17424e to 7b09018 Compare November 7, 2024 16:01
@Fajfa Fajfa force-pushed the 2024.9.x branch 2 times, most recently from 5b052f0 to dd50534 Compare November 22, 2024 10:05
@Fajfa Fajfa force-pushed the 2024.9.x branch 3 times, most recently from 94f78e4 to 628d0da Compare December 6, 2024 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fajfa Fajfa Fajfa approved these changes

@tjerman tjerman tjerman approved these changes

Assignees
No one assigned
Labels
no-pr-activity
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add option to make and use global blocks
6 participants
@kelanik8 @katrinDY @tjerman @Fajfa @EmmyMay @KinyaElGrande