Skip to content

Fix image alt text and word css contracts - #44

Draft
HassanAkbar wants to merge 4 commits into
mainfrom
fix/todo-02-spec-doubles
Draft

Fix image alt text and word css contracts#44
HassanAkbar wants to merge 4 commits into
mainfrom
fix/todo-02-spec-doubles

Conversation

@HassanAkbar

Copy link
Copy Markdown
Member

Metanorma PR checklist

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR aligns accessibility and MHTML/CSS generation logic with Uniword’s real OOXML model contracts, and updates specs to exercise production parsing paths instead of relying on RSpec doubles.

Changes:

  • Fix ImageAltTextRule to read image alt text from wp:docPr/@descr on real Wordprocessingml::Drawing frames and update related accessibility specs to parse real OOXML.
  • Fix Mhtml::WordCss to consume Wordprocessingml::Style’s actual API (id, font_family, alignment.value) and add Style#italic plus shared ST_OnOff boolean handling.
  • Add/expand specs around ST_OnOff token semantics and update the TODO note tracking spec-double removal progress.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
TODO/02-reduce-spec-doubles.md Updates the project TODO/status narrative and documents the resolved contract issues.
spec/uniword/wordprocessingml/styles_spec.rb Adds targeted specs proving Style ST_OnOff readers delegate consistently.
spec/uniword/properties/bold_spec.rb Adds a comprehensive ST_OnOff lexical table spec for boolean element parsing.
spec/uniword/mhtml/word_css_spec.rb Replaces style/numbering doubles with real parsed WordprocessingML configs and adds coverage for font-size and toggle semantics.
spec/uniword/accessibility/rules/image_alt_text_rule_spec.rb Replaces image doubles with parsed OOXML drawings and adds coverage for inline/anchor and fallback behavior.
spec/uniword/accessibility/rules_integration_spec.rb Updates integration spec to use a parsed document containing a drawing without descr.
spec/uniword/accessibility/accessibility_checker_spec.rb Updates checker spec to use a parsed document containing a drawing without descr.
lib/uniword/wordprocessingml/style.rb Refactors boolean readers via a shared helper and adds #italic.
lib/uniword/properties/boolean_element_factory.rb Adds ST_OnOff-aware #on? to boolean element wrappers.
lib/uniword/mhtml/word_css.rb Updates build_style_rule to match Wordprocessingml::Style and uses half-point font-size formatting.
lib/uniword/accessibility/rules/image_alt_text_rule.rb Switches rule logic from image.alt_text to extracting wp:docPr/@descr from drawings.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/uniword/properties/boolean_element_factory.rb Outdated
Comment thread lib/uniword/accessibility/rules/image_alt_text_rule.rb Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

spec/uniword/accessibility/rules_integration_spec.rb:148

  • This example name says "disabled rules return no violations" but the spec only asserts enabled? and never checks violations. Renaming the example (or adding an assertion on the returned violations) would prevent misleading test output.
    it "disabled rules return no violations" do

TODO/02-reduce-spec-doubles.md:19

  • Line reads as an unfinished sentence ("...PR 1 and PR 2 are correctness"), which makes the TODO harder to interpret. Consider completing it (e.g., "correctness fixes") and adding punctuation.
**Priority:** Medium (spec quality), but PR 1 and PR 2 are correctness
**Files:** see PR breakdown below

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 49 out of 49 changed files in this pull request and generated no new comments.

Suppressed comments (1)

spec/uniword/wordprocessingml/update_fields_spec.rb:25

  • The test says ST_OnOff “spells off as w:val="false"” and BooleanValSetter normalizes Ruby false to that spelling, but the assertion currently allows other tokens (0/off). This makes the spec less strict than the implementation and could hide regressions in the intended serialization format.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants