refactor!: Rename name to label and use value for label content
#475
Conversation
|
I'm on board with these changes, but I expect the transition to the value property to be painful. Maybe you should split this PR so it has its dedicated entry in the changelog, until we properly document everything? Also, you're effectively creating a new kind of nodes which are labeled by their value, and I wonder if you should explicitly name them by adding a new method in the consumer crate. I currently have no idea how we'd name this class of nodes, but I don't really like having this check on roles in multiple places. |
|
I don't think I understand the benefit of splitting the PR. |
|
It's a subtle change and I wouldn't be surprised if people don't set the value property on labels while migrating, so I'm just trying to see how to make this change more obvious. I think having a dedicated entry in the changelog would make it more noticeable. |
|
I don't like the extra churn that splitting the PR would cause in the code itself, just for one extra line in the change log. |
|
Thanks for adding documentation. It doesn't mention that it also applies to |
|
Maybe this is to allow a rich text alternative? |
|
You're right about |
mwcampbell
force-pushed
the
refactor-label
branch
from
b05c5bb
to
c73b612
Compare
# Objective AccessKit/accesskit#475 changed how text content should be set for AccessKit nodes with a role of `Label`. This was unfortunately missing from #16234. ## Solution When building an `accesskit::Node` with `Role::Label`, calls `set_value` instead of `set_label` on the node to set its content. ## Testing I can't test this right now on my Windows machine due to a compilation error with wgpu-hal I have no idea how to resolve.
# Objective AccessKit/accesskit#475 changed how text content should be set for AccessKit nodes with a role of `Label`. This was unfortunately missing from #16234. ## Solution When building an `accesskit::Node` with `Role::Label`, calls `set_value` instead of `set_label` on the node to set its content. ## Testing I can't test this right now on my Windows machine due to a compilation error with wgpu-hal I have no idea how to resolve.
# Objective AccessKit/accesskit#475 changed how text content should be set for AccessKit nodes with a role of `Label`. This was unfortunately missing from bevyengine#16234. ## Solution When building an `accesskit::Node` with `Role::Label`, calls `set_value` instead of `set_label` on the node to set its content. ## Testing I can't test this right now on my Windows machine due to a compilation error with wgpu-hal I have no idea how to resolve.
I think the new property name and the new use of
valueon nodes withRole::Labelmakes sense because:labelproperty, or thelabelled_byrelation. The word choice is more consistent.TextRunnodes, we can now automatically get the label's value from its descendant text runs, rather than having to set the value directly; this follows automatically from the existing support for constructing the value from text inaccesskit_consumer::Node::value.