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.3.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
2,127 −1,239

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

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 and others added 2 commits April 15, 2024 15:19
@kelanik8 @Fajfa
Add visibility condition to page blocks
7b15493
@EmmyMay @Fajfa
Standardize list inputs
1a248c7 
***Refactor field row view component in module***
***Add small responsive design to unique values table***
***Update system fields table in module edit view
@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tjerman tjerman tjerman approved these changes

@Fajfa Fajfa Fajfa 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