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

Remove styles #185

Merged
merged 3 commits into from
Oct 19, 2023
Merged

Remove styles #185

merged 3 commits into from
Oct 19, 2023

Conversation

cconcolato
Copy link
Contributor

@cconcolato cconcolato commented Oct 4, 2023

Close #124
Close #162


Preview | Diff

@cconcolato
Copy link
Contributor Author

The PR does not contain the modified SVG file. PlantUML does not want to generate a nice one on my machine. @nigelmegitt can you add it?

@himorin
Copy link
Contributor

himorin commented Oct 5, 2023

I could not identify from where build error coming... It says html validation failed as:

"file:/home/runner/work/dapt/dapt.w3c/index.html":803.19618-803.19917: error: Attribute “title” not allowed on element “a” at this point.

"file:/home/runner/work/dapt/dapt.w3c/index.html":803.20843-803.21141: error: Attribute “title” not allowed on element “a” at this point.
"file:/home/runner/work/dapt/dapt.w3c/index.html":803.22877-803.23177: error: Attribute “title” not allowed on element “a” at this point.

but with exported html from respec, nu html checker reported no error, nor I could not find title (not xlink:title) around reported line...
It seems similar to one we had initially with SVG image, but it does not change any SVG file at this point in this PR.

@cconcolato
Copy link
Contributor Author

@himorin I think the error is due to the SVG file referring to some sections of the spec that have been removed. Once the SVG is regenerated (my request to Nigel, above), that should be fine.

@nigelmegitt
Copy link
Contributor

Will do!

@nigelmegitt
Copy link
Contributor

Done, and the checks passed this time, confirming @cconcolato's hypothesis.

Copy link
Contributor

@nigelmegitt nigelmegitt left a comment

Choose a reason for hiding this comment

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

Looks great, just one suggestion for completeness, but modulo that, approving.

index.html Outdated Show resolved Hide resolved
nigelmegitt added a commit that referenced this pull request Oct 6, 2023
Also pre-emptively remove the Style assuming that #185 is approved too. If not, will need to re-instate Style.
Copy link

@andreastai andreastai left a comment

Choose a reason for hiding this comment

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

Looks good to me

nigelmegitt added a commit that referenced this pull request Oct 11, 2023
* Split Script Event Description out as a separate object

Closes #174.

Clarifies the cardinality of Script Event Description.

Updates the data model diagram (in a way that is non-ideal but hard to fix automatically: it may be worth replacing the PlantUML-generated diagram with a more manually constructed one prior to final publication), and defines Description Type more explicitly.

Also clarify that there is no uniqueness constraint for `daptm:descType` and that `xml:lang` can be used to label the language of the contents of a `ttm:desc`.

* Add optional language to script description in UML model

Also pre-emptively remove the Style assuming that #185 is approved too. If not, will need to re-instate Style.

* Address review feedback

* typos and minor editorial tweaks
@nigelmegitt nigelmegitt merged commit 4988a61 into main Oct 19, 2023
2 checks passed
@nigelmegitt nigelmegitt deleted the style_removal branch October 19, 2023 13:46
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.

Can Script Events be styled? Reconsidering the role of character styles
4 participants