-
Notifications
You must be signed in to change notification settings - Fork 0
feat: initial implementation #10
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
Draft
javier-godoy
wants to merge
6
commits into
master
Choose a base branch
from
first-iteration
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
219354b
feat: initial implementation
javier-godoy 12bc9bb
WIP: apply coderabbit suggestions
javier-godoy 0a8e5f1
WIP: apply coderabbit suggestions
javier-godoy ffa881e
WIP: refactor so that code rabbit doesn't complain
javier-godoy 48cba73
WIP: simplify birthday calculation in demo code
javier-godoy 031a694
WIP: self review
javier-godoy File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| # Configuration Resolution: Scope-First vs. Type-First | ||
|
|
||
| ## The Two Strategies | ||
|
|
||
| Given `Foo extends Entity` with configurations at four points in the matrix: | ||
|
|
||
| | | Instance | Global | | ||
| |---|---|---| | ||
| | **Foo** | I·Foo | G·Foo | | ||
| | **Entity** | I·Entity | G·Entity | | ||
|
|
||
| **Scope-first** exhausts all instance settings before looking at global ones: | ||
| ``` | ||
| I·Foo → I·Entity → G·Foo → G·Entity → Default | ||
| ``` | ||
|
|
||
| **Type-first** exhausts all Foo-specific settings before looking at Entity: | ||
| ``` | ||
| I·Foo → G·Foo → I·Entity → G·Entity → Default | ||
| ``` | ||
|
|
||
| Both agree on `I·Foo` being first and `G·Entity` being last. The dispute is the middle two. | ||
|
|
||
| --- | ||
|
|
||
| ## Where Scope-First Makes Sense | ||
|
|
||
| **Setting: `textAlign` or `nullRepresentation`** | ||
|
|
||
| A developer creates a reporting grid and writes: | ||
| ```java | ||
| easyGrid.configuration.forType(Entity.class).setTextAlign(RIGHT); | ||
| ``` | ||
| The intent is *"every Entity-like column in this grid should be right-aligned."* Since `Foo IS-AN Entity`, Foo columns should also be right-aligned. Instance context wins: the developer explicitly opted every Entity column into this layout decision, and Foo, being substitutable for Entity, should inherit it. | ||
|
|
||
| If type-first were applied, `G·Foo` would sit between `I·Foo` and `I·Entity`. An unrelated global renderer for Foo would silently block the instance-level alignment — which is the opposite of what the developer intended. | ||
|
|
||
| --- | ||
|
|
||
| ## Where Type-First Makes Sense | ||
|
|
||
| **Setting: `rendererFactory`** | ||
|
|
||
| Globally, `Foo` has a specialized renderer registered: | ||
| ```java | ||
| GlobalEasyGridConfiguration.forType(Foo.class) | ||
| .setRendererFactory(FooRenderers.of(...)); | ||
| ``` | ||
| Independently, an instance configures Entity columns with a generic formatter: | ||
| ```java | ||
| easyGrid.configuration.forType(Entity.class).setFormatter(e -> e.getId().toString()); | ||
| ``` | ||
|
|
||
| Under scope-first, `I·Entity` wins over `G·Foo`. The Foo-specific renderer — which encodes *what a Foo looks like*, arguably a type invariant — gets silently replaced by a generic Entity formatter that the developer may not have intended to apply to Foo. | ||
|
|
||
| Type-first preserves the type contract: `G·Foo` sits above `I·Entity`, so Foo's rendering semantics are honoured even across scope boundaries. | ||
|
|
||
| --- | ||
|
|
||
| ## Liskov Substitution Principle | ||
|
|
||
| LSP says: a `Foo` must be usable wherever an `Entity` is expected, without callers needing to know the difference. | ||
|
|
||
| Applied to configuration, it has two implications that pull in opposite directions: | ||
|
|
||
| **LSP supports scope-first for behavioural settings.** If you configure `Entity` at instance level — null representation, alignment, a locale-specific formatter — you are specifying how *this grid* handles all Entity values. A `Foo` substituting for `Entity` should satisfy those same observable postconditions. Refusing to inherit `I·Entity` because Foo has a `G·Foo` entry would mean the grid behaves differently for `Foo` vs `Entity` in ways the calling code did not anticipate. | ||
|
|
||
| **LSP supports type-first for type-defining settings.** LSP also requires that subtypes honour their own invariants. If `Foo` has a type-level rendering contract — *this is how a Foo is displayed* — then substituting a generic `Entity` renderer for it violates the Foo invariant. The renderer is not just a postcondition on the grid; it is a property of `Foo` itself. | ||
|
|
||
| The tension resolves in favour of **scope-first for all properties**. The type-first argument rests on the assumption that `I·Entity` displaces `G·Foo` unintentionally — but instance configuration is always deliberate. A developer who writes `forType(Entity.class).setFormatter(...)` knows `Foo IS-AN Entity`; if they wanted Foo to keep its global renderer they would have set `I·Foo` separately. The argument collapses entirely at broad overrides such as `forType(Object.class).setFormatter(...)`, where the developer has unambiguously stated that everything in this grid uses this formatter and global type-specific registrations cannot claim precedence. **Global registrations are defaults. Instance registrations are decisions. Decisions outrank defaults regardless of type specificity.** | ||
|
|
||
| --- | ||
|
|
||
| ## Current Implementation | ||
|
|
||
| The implementation uses **scope-first** for all properties. For a `Foo extends Entity` column the effective chain is: | ||
|
|
||
| ``` | ||
| per-column → I·Foo → I·Entity → I·Object → G·Foo → G·Entity → G·Object → Default | ||
| ``` | ||
|
|
||
| This is built in two pieces: | ||
|
|
||
| **Instance chain** — `EasyGridConfigurationClassMap.getOrCreate(Foo)` walks the Java class hierarchy within the same map, producing a `ColumnConfigurationImpl` chain: | ||
|
|
||
| ``` | ||
| I·Foo(impl) → I·Entity(impl) → I·Object(impl) | ||
| ``` | ||
|
|
||
| **Global chain** — `GlobalEasyGridConfiguration.resolve(Foo)` similarly produces: | ||
|
|
||
| ``` | ||
| G·Foo(impl) → G·Entity(impl) → G·Object(impl) | ||
| ``` | ||
|
|
||
| **Bridge** — `InstanceEasyGridConfiguration.forType(Foo)` wraps both into a `ColumnConfigurationLink`: | ||
|
|
||
| ``` | ||
| ColumnConfigurationLink(primary = I·Foo chain, fallback = G·Foo chain) | ||
| ``` | ||
|
|
||
| `ColumnConfigurationLink.get()` consults the primary chain first and only falls back to the global chain when the primary returns `null` for a given property. | ||
|
|
||
| **Per-column** — `InstanceEasyGridConfiguration.resolve(Foo)` wraps the link in a fresh `ColumnConfigurationImpl` whose fields are overridden by column-level setters (`setNullRepresentation`, `setFormatter`, `setRendererFactory`). | ||
|
|
||
| **Default** — when the entire chain returns `null` for `getRendererFactory()`, `EasyColumn.createRenderer` applies a `ColumnConfigurationTextRenderer` with null-representation support as the last resort. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| # Feature: Row Actions | ||
|
|
||
| Row actions are buttons or menu items displayed in a dedicated actions column, created and managed by `EasyGrid` on the wrapped grid. | ||
|
|
||
| ## API | ||
|
|
||
| ### `EasyGrid<T>` methods | ||
|
|
||
| ```java | ||
| // Add an action button (label + icon) | ||
| EasyRowAction<T> addRowAction(String label, VaadinIcon icon, SerializableConsumer<T> handler); | ||
|
|
||
| // Add an action button with a theme variant | ||
| EasyRowAction<T> addRowAction(String label, VaadinIcon icon, ButtonVariant variant, SerializableConsumer<T> handler); | ||
|
|
||
| // Render all actions as a context menu (overflow menu) instead of inline buttons | ||
| void setRowActionsAsMenu(boolean asMenu); | ||
|
|
||
| // Access the underlying Grid.Column for header, width, freezing, etc. | ||
| Grid.Column<T> getActionsColumn(); | ||
| ``` | ||
|
|
||
| ### `EasyRowAction<T>` | ||
|
|
||
| ```java | ||
| public class EasyRowAction<T> { | ||
| // Conditional visibility | ||
| EasyRowAction<T> withVisibleWhen(SerializablePredicate<T> predicate); | ||
|
|
||
| // Conditional enablement | ||
| EasyRowAction<T> withEnabledWhen(SerializablePredicate<T> predicate); | ||
|
|
||
| // Tooltip | ||
| EasyRowAction<T> withTooltip(String tooltip); | ||
| EasyRowAction<T> withTooltip(SerializableFunction<T, String> tooltipProvider); | ||
|
|
||
| // Confirmation dialog before executing the action | ||
| EasyRowAction<T> withConfirmation(String message); | ||
| EasyRowAction<T> withConfirmation(String title, String message); | ||
| } | ||
| ``` | ||
|
|
||
| ## Usage | ||
|
|
||
| ```java | ||
| // Inline action buttons | ||
| easyGrid.addRowAction("Edit", VaadinIcon.EDIT, person -> { | ||
| editPerson(person); | ||
| }); | ||
|
|
||
| easyGrid.addRowAction("Delete", VaadinIcon.TRASH, ButtonVariant.LUMO_ERROR, person -> { | ||
| personService.delete(person); | ||
| easyGrid.getDataProvider().refreshAll(); | ||
| }).withConfirmation("Are you sure you want to delete this person?"); | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| // Actions as a context menu (overflow menu) instead of inline buttons | ||
| easyGrid.setRowActionsAsMenu(true); | ||
|
|
||
| // Conditional visibility | ||
| easyGrid.addRowAction("Activate", VaadinIcon.CHECK, person -> { | ||
| personService.activate(person); | ||
| }).withVisibleWhen(person -> !person.isActive()); | ||
|
|
||
| easyGrid.addRowAction("Deactivate", VaadinIcon.CLOSE, person -> { | ||
| personService.deactivate(person); | ||
| }).withVisibleWhen(person -> person.isActive()); | ||
|
|
||
| // Configure the actions column via the underlying Grid.Column | ||
| easyGrid.getActionsColumn() | ||
| .setHeader("Actions") | ||
| .setWidth("150px") | ||
| .setFrozenToEnd(true); | ||
| ``` | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add language identifiers to fenced code blocks.
Line 13, Line 18, Line 78, Line 86, Line 92, and Line 98 use unlabeled fences, which triggers MD040 and can fail markdown lint gates.
Suggested fix
-
+textI·Foo → G·Foo → I·Entity → G·Entity → Default
-
+textI·Foo(impl) → I·Entity(impl) → I·Object(impl)
-
+textColumnConfigurationLink(primary = I·Foo chain, fallback = G·Foo chain)
Also applies to: 78-100
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 13-13: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 18-18: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Defer to reviewers' judgment. This file is intended for AI consumption.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@javier-godoy, understood — I'll dismiss this concern. SinceCONFIGURATION_RESOLUTION.mdis intended for AI consumption rather than human readers or standard documentation pipelines, markdown lint compliance is not a meaningful concern here.✏️ Learnings added