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

Evaluate if the embedded code examples have a visible scrollbar #2450

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
105 changes: 64 additions & 41 deletions mdbook-slide-evaluator/src/evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,6 @@ impl From<(f64, f64, f64, f64)> for ElementSize {
pub struct EvaluationResult {
/// metadata about the slide
slide: Slide,
/// the size of the main content element
element_size: ElementSize,
/// all policy violations
policy_violations: Vec<PolicyViolation>,
}
Expand All @@ -85,8 +83,6 @@ pub struct EvaluationResults {
#[derive(Serialize)]
struct ExportFormat {
filename: PathBuf,
element_width: usize,
element_height: usize,
policy_violations: String,
}

Expand All @@ -113,8 +109,6 @@ impl EvaluationResults {
}
csv_writer.serialize(ExportFormat {
filename: (*result.slide.filename).to_path_buf(),
element_width: result.element_size.width.round() as usize,
element_height: result.element_size.height.round() as usize,
policy_violations: result
.policy_violations
.iter()
Expand All @@ -133,10 +127,8 @@ impl EvaluationResults {
continue;
}
println!(
"{}: {}x{} [{}]",
"{}: [{}]",
result.slide.filename.display(),
result.element_size.width,
result.element_size.height,
result
.policy_violations
.iter()
Expand Down Expand Up @@ -181,23 +173,6 @@ impl<'a> Evaluator<'_> {
Ok(())
}

/// evaluate the currently opened webpage return the selected content
/// element if available
async fn get_content_element_from_slide(
&self,
) -> anyhow::Result<Option<Element>> {
match self.webclient.find(self.element_selector).await {
Result::Ok(result) => Ok(Some(result)),
Result::Err(fantoccini::error::CmdError::Standard(
fantoccini::error::WebDriver {
error: fantoccini::error::ErrorStatus::NoSuchElement,
..
},
)) => anyhow::Ok(None),
Result::Err(error) => Err(anyhow!(error))?,
}
}

/// extract the element coordinates from this element
async fn get_element_coordinates(
&self,
Expand Down Expand Up @@ -235,6 +210,58 @@ impl<'a> Evaluator<'_> {
Ok(())
}

/// evaluates the main element of the page if there is one and returns violations of the policy
async fn eval_main_element(
&self,
slide: &Slide,
) -> anyhow::Result<Vec<PolicyViolation>> {
// get every main content element - should only be one but find_all makes
// the code easier as we just care about violations of any main element
let content_elements =
self.webclient.find_all(self.element_selector).await?;

let mut violations = vec![];
for element in content_elements {
let element_size = self.get_element_coordinates(&element).await?;
violations.append(&mut self.slide_policy.eval_size(&element_size));
if self.screenshot_dir.is_some() {
let screenshot = element.screenshot().await?;
self.store_screenshot(screenshot, &slide.filename)?;
}
}
Ok(violations)
}

/// Determines if the current page has a code example (using ACE) that has
/// a scrollbar by evaluating if the ace_scrollbar-[hv] element is displayed
async fn eval_code_example_scrollbar(
&self,
) -> anyhow::Result<Vec<PolicyViolation>> {
let v_scrollbar_elements_path = fantoccini::Locator::XPath(
r#"//*[@id="content"]//div[contains(@class, "ace_scrollbar-v")]"#,
);
let h_scrollbar_elements_path = fantoccini::Locator::XPath(
r#"//*[@id="content"]//div[contains(@class, "ace_scrollbar-h")]"#,
);
let v_scrollbar_elements =
self.webclient.find_all(v_scrollbar_elements_path).await?;
let h_scrollbar_elements =
self.webclient.find_all(h_scrollbar_elements_path).await?;

let mut violations = vec![];
for element in v_scrollbar_elements {
if element.is_displayed().await? {
violations.push(PolicyViolation::CodeExampleVScrollbar);
}
}
for element in h_scrollbar_elements {
if element.is_displayed().await? {
violations.push(PolicyViolation::CodeExampleHScrollbar);
}
}
Ok(violations)
}

/// evaluate a single slide
pub async fn eval_slide(
&self,
Expand All @@ -245,21 +272,13 @@ impl<'a> Evaluator<'_> {
let url = self.html_base_url.join(&slide.filename.display().to_string())?;
self.webdriver_open_url(&url).await?;

let Some(content_element) = self.get_content_element_from_slide().await?
else {
return Ok(None);
};
let element_size = self.get_element_coordinates(&content_element).await?;
if self.screenshot_dir.is_some() {
let screenshot = content_element.screenshot().await?;
self.store_screenshot(screenshot, &slide.filename)?;
}
let policy_violations = self.slide_policy.eval_size(&element_size);
let result = EvaluationResult {
slide: slide.clone(),
element_size,
policy_violations,
};
let mut policy_violations = vec![];
// evaluate main content element size
policy_violations.append(&mut self.eval_main_element(slide).await?);
// evaluate code examples size
policy_violations.append(&mut self.eval_code_example_scrollbar().await?);

let result = EvaluationResult { slide: slide.clone(), policy_violations };
debug!("information about element: {:?}", result);
Ok(Some(result))
}
Expand Down Expand Up @@ -290,6 +309,10 @@ enum PolicyViolation {
MaxWidth,
/// violation of the maximum width
MaxHeight,
/// code examples should not contain a vertical scrollbar
CodeExampleVScrollbar,
/// code examples should not contain a horizontal scrollbar
CodeExampleHScrollbar,
}

/// the SlidePolicy struct contains all parameters for evaluating a slide
Expand Down
6 changes: 5 additions & 1 deletion mdbook-slide-evaluator/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ struct Args {
violations_only: bool,
/// directory of the book that is evaluated
source_dir: PathBuf,
/// ignore HTML pages that redirect to canonical pages
#[arg(long, default_value_t = false)]
ignore_redirects: bool,
}

#[tokio::main]
Expand All @@ -71,7 +74,8 @@ async fn main() -> anyhow::Result<()> {
let args = Args::parse();

// gather information about the book from the filesystem
let book = Book::from_html_slides(args.source_dir.clone())?;
let book =
Book::from_html_slides(args.source_dir.clone(), args.ignore_redirects)?;

// create a new webclient that is used by the evaluator
let webclient: fantoccini::Client =
Expand Down
33 changes: 31 additions & 2 deletions mdbook-slide-evaluator/src/slides.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,12 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use std::fs::File;
use std::io::Read;
use std::path::{Path, PathBuf};
use std::sync::Arc;

use anyhow::Ok;
use log::debug;

/// a slide is a page in the book
Expand All @@ -33,17 +36,26 @@ pub struct Book {

impl Book {
/// create a book from all html files in the source_dir
pub fn from_html_slides(source_dir: PathBuf) -> anyhow::Result<Book> {
pub fn from_html_slides(
source_dir: PathBuf,
ignore_redirects: bool,
) -> anyhow::Result<Book> {
let mut slides = vec![];
let files = glob::glob(&format!(
"{}/**/*.html",
source_dir.to_str().expect("invalid path")
))?;
for file in files {
let slide = Slide { filename: file?.into() };
let file = file?;
if ignore_redirects && file_is_redirect(&file)? {
debug!("slide {file:?} is a redirect page");
continue;
}
let slide = Slide { filename: file.into() };
debug!("add {:?}", slide);
slides.push(slide);
}
debug!("processing {} slides", slides.len());
Ok(Book { _source_dir: source_dir, slides })
}

Expand All @@ -52,3 +64,20 @@ impl Book {
&self.slides
}
}

const HTML_REDIRECT_PAGE: &str = r#"<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Redirecting...</title>"#;

/// check if the file is starting with the mdbook redirect page.
/// This method is optimized to not read the entire file but only the start
fn file_is_redirect(filename: &PathBuf) -> anyhow::Result<bool> {
let mut file = File::open(filename)?;
// create a buffer with the exact length of the text that is checked
let mut file_start_buffer = [0u8; HTML_REDIRECT_PAGE.len()];
// read only the part that is relevant
file.read_exact(&mut file_start_buffer)?;
Ok(file_start_buffer.eq(HTML_REDIRECT_PAGE.as_bytes()))
}