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

Implements page #559

Merged
merged 2 commits into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,17 @@ jobs:
uses: ruby/setup-ruby@v1
with:
bundler-cache: true
- name: Run Lacci tests
run: CI_RUN='true' bundle exec rake lacci_test
# Currently failing
# - name: Run Lacci tests
# run: CI_RUN='true' bundle exec rake lacci_test
Copy link
Collaborator

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?

Copy link
Collaborator Author

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

- name: Run Scarpe-Component tests
run: CI_RUN='true' bundle exec rake component_test
- name: Run Scarpe tests
run: CI_RUN='true' bundle exec rake test
- name: Check HTML output
run: bundle exec rake test:check_html_fixtures
- name: upload test-fail logs
if: '!cancelled()'
if: !cancelled()
uses: actions/upload-artifact@v4
with:
name: test failure logs
Expand Down
33 changes: 33 additions & 0 deletions docs/SCARPE_FEATURES.md
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
```
Binary file added docs/image.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion examples/border.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@
para "This border is also on top of text"
border blue, strokewidth: 4
end
end
end
42 changes: 42 additions & 0 deletions examples/url_navigation_single_app.rb
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
26 changes: 25 additions & 1 deletion lacci/lib/shoes/app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ def initialize(

@slots = []

@content_container = nil

super

# This creates the DocumentRoot, including its corresponding display drawable
Expand Down Expand Up @@ -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)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This automatically makes a new flow around every app?

Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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"

Copy link
Collaborator

Choose a reason for hiding this comment

The 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
Expand Down Expand Up @@ -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

Expand Down
20 changes: 9 additions & 11 deletions lacci/lib/shoes/drawable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]
Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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

Expand Down