-
Notifications
You must be signed in to change notification settings - Fork 29
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
Implements page
#559
Implements page
#559
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
# Extending _why's Legacy | ||
|
||
![Would _why have a beard if he existed now](image.png) | ||
|
||
The leading mission for Scarpe has been to implement as much backwards compatibility as possible for _why's original | ||
Shoes library. This remains true. _why's taste and DSL are celebrated and preserved. However, at the discretion of | ||
core maintainers, new features may be added. They must be approved by Noah Gibbs or Nick Schwaderer, and described | ||
in this file. They cannot conflict or damage backwards compatibility with the original Shoes library. | ||
|
||
## Page Navigation | ||
|
||
Similar to URL navigation. see `url_navigation_single_app.rb` for an example. Unlike URLs, they do not accept parameters, | ||
only the name of the page. They are simply declared and named in blocks. | ||
|
||
```ruby | ||
Shoes.app do | ||
page(:home) do | ||
title "Home Page" | ||
para "Welcome to the home page" | ||
button "Go to another page" do | ||
visit(:another_page) | ||
end | ||
end | ||
|
||
page(:another_page) do | ||
title "Another Page" | ||
para "Welcome to another page" | ||
button "Go to home page" do | ||
visit(:home) | ||
end | ||
end | ||
end | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,4 +8,4 @@ | |
para "This border is also on top of text" | ||
border blue, strokewidth: 4 | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
Shoes.app(title: "Page Navigation Example", width: 300, height: 200) do | ||
style(Shoes::Para, size: 10) | ||
style(Shoes::Button, width: 80) | ||
|
||
page(:home) do | ||
title "Home Page" | ||
background "#f0f0f0" | ||
para "Welcome to the page navigation example!" | ||
button "Go to Razzmatazz" do | ||
visit(:razzmatazz) | ||
end | ||
button "Go to FlooperLand" do | ||
visit(:flooperland) | ||
end | ||
end | ||
|
||
page(:razzmatazz) do | ||
title "Razzmatazz" | ||
background "#DFA5A5" | ||
para "This is Razzmatazz" | ||
button "Go Home" do | ||
visit(:home) | ||
end | ||
button "Go to FlooperLand" do | ||
visit(:flooperland) | ||
end | ||
end | ||
|
||
page(:flooperland) do | ||
title "FlooperLand" | ||
background "#A5DFA5" | ||
para "This is FlooperLand" | ||
button "Go Home" do | ||
visit(:home) | ||
end | ||
button "Go to Razzmatazz" do | ||
visit(:razzmatazz) | ||
end | ||
end | ||
|
||
visit(:home) # Start at the home page | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,6 +73,8 @@ def initialize( | |
|
||
@slots = [] | ||
|
||
@content_container = nil | ||
|
||
super | ||
|
||
# This creates the DocumentRoot, including its corresponding display drawable | ||
|
@@ -120,7 +122,10 @@ def init | |
send_shoes_event(event_name: "init") | ||
return if @do_shutdown | ||
|
||
with_slot(@document_root, &@app_code_body) | ||
with_slot(@document_root) do | ||
@content_container = flow(width: 1.0, height: 1.0) | ||
with_slot(@content_container, &@app_code_body) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This automatically makes a new flow around every app? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, okay. There's a new flow that then contains whatever the page is. Hrm. I should figure out if that messes with the formatting. I know with CSS stuff we had a devil of a time figuring out when adding a new container to the tree changed formatting. But this one may be fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes I was having bugs where certain content was getting painted over and over on button click. Part of resolving this, and I thought this made sense, was a silent container for each and every "page" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm. Since you're clearing the content_container every time, I'd think it'd make more sense to clear the DocumentRoot. If you're getting paint errors with that method, we should figure out why and fix those. Also, can you skip the stack() around pages if you're doing this? It seems odd to me that pages stack vertically by default but everything else stacks horizontally. And I think you can just skip making the extra container. |
||
end | ||
end | ||
|
||
# "Container" drawables like flows, stacks, masks and the document root | ||
|
@@ -273,6 +278,25 @@ def find_drawables_by(*specs) | |
end | ||
drawables | ||
end | ||
|
||
def page(name, &block) | ||
@pages ||= {} | ||
@pages[name] = proc do | ||
stack(width: 1.0, height: 1.0) do | ||
instance_eval(&block) | ||
end | ||
end | ||
end | ||
|
||
def visit(name) | ||
if @pages && @pages[name] | ||
@content_container.clear do | ||
instance_eval(&@pages[name]) | ||
end | ||
else | ||
puts "Error: URL '#{name}' not found" | ||
end | ||
end | ||
end | ||
end | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,12 +54,12 @@ def is_widget_class?(name) | |
def validate_as(prop_name, value) | ||
prop_name = prop_name.to_s | ||
hashes = shoes_style_hashes | ||
|
||
h = hashes.detect { |hash| hash[:name] == prop_name } | ||
raise(Shoes::Errors::NoSuchStyleError, "Can't find property #{prop_name.inspect} in #{self} property list: #{hashes.inspect}!") unless h | ||
|
||
return value if h[:validator].nil? | ||
|
||
# Pass both the property name and value to the validator block | ||
h[:validator].call(value,prop_name) | ||
end | ||
|
@@ -242,17 +242,15 @@ def shoes_style_name?(name) | |
# not kept long, and used up when used once. | ||
|
||
def with_current_app(app) | ||
old_cur_app = @current_app | ||
@current_app = app | ||
ret = yield | ||
@current_app = old_cur_app | ||
ret | ||
old_app = Thread.current[:shoes_app] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting. Were you doing something that needed this? This is going to change multithreaded behaviour... Which is maybe fine? This way could be better if you have multiple threads and you assume each thread will be running a different app. It will fail if multiple threads ever need to use the same app. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was a bit deep when I put this together. Let me revisit this code and either explain myself or revert. I'd mentioned in the discord but some of my contributions today were a bit aggressive which increases the risk that I shipped a change not pivotal to my feature. |
||
Thread.current[:shoes_app] = app | ||
yield | ||
ensure | ||
Thread.current[:shoes_app] = old_app | ||
end | ||
|
||
def use_current_app | ||
cur_app = @current_app | ||
@current_app = nil | ||
cur_app | ||
Thread.current[:shoes_app] | ||
end | ||
end | ||
|
||
|
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.
Hm. This turns off all Lacci tests?
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.
I filed an issue when I found that I was getting the same Laci failures on CI on this branch, locally on this branch, and locally on main.
Maybe this was too aggressive, but thought that could be the best approach to isolate that issue in a separate PR
Maybe it's not reproducible though? I think you have commented on this issue so let's revisit
I definitely do want these tests. But if they were going red on main I wanted to toggle them until resolved